Re: [PATCH][BZ 19084] Make sure _nl_value_type_LC_<category> arrays have correct size.

[Rafal Luzynski <digitalfreak at lingonborough dot com>]

Mike,

What is the status of my patch [1] and the discussion below? I think you
expressed some positive opinion about it. I don't insist on accepting
and pushing it but if you accept it then I'd like to know because then
I should remove this change from my patches against bug 10871 [2] [3].

Regards,

Rafal

[1] https://sourceware.org/ml/libc-alpha/2016-03/msg00521.html
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=10871
[3] https://sourceware.org/ml/libc-alpha/2016-03/msg00616.html


11.04.2016 22:02 Mike Frysinger <vapier@gentoo.org> wrote:
>
> On 11 Apr 2016 21:35, Rafal Luzynski wrote:
> > 10.04.2016 o 06:28 Mike Frysinger wrote:
> > > thanks, your explanation clears things up. your patch isn't related to
> > > bug 19084 though as we discussed. i'm fine with this patch then. how
> > > about this commit message though:
> > >
> > > When constructing the locale arrays, we use categories.def with macros.
> > > For stringarrays (like ABDAY_1), we end up only populating the first of
> > > the array and leave holes for the rest (i.e. we omit ABDAY_2, etc...).
> > > When there happen to be more items after it, things work out because we
> > > implicitly pad out the array to cover the omitted elements. However,
> > > if these elements are specified last, we don't pad things out, and the
> > > internal checks reject the compiled locale.
> > >
> > > For example, if ABDAY_1 came last, the array would look like:
> > > static const enum value_type _nl_value_type_LC_TIME[] = {
> > > ...
> > > [...ABDAY_1...] = stringarray,
> > > };
> > > Trying to index ABDAY_2 would now run off the end of the array.
> >
> > Yes, I agree with this.
> >
> > A legal notice: I have not (yet!) signed any copyright assignment with
> > FSF but such a short update is not considered as legally significant [1]
> > so does not need any assignment. Also, if in doubt, here I state that
> > I will not complain if you (or any other person) commit, reject, change,
> > or do anything you find appropriate with this change.
>
> ok, for this less-than-one-line change, it's OK. but for your other
> patches (abmon stuff), we'll need CLA papers in place.
>
> > By the way, do you think that after this patch you can stop skipping
> > the array size test for LC_TYPE? I wrote:
> >
> > > A similar problem is in LC_CTYPE category, it ends with a subarray of
> > > _NL_CTYPE_EXTRA_MAP_1,..,_NL_CTYPE_EXTRA_MAP_14. Probably this is
> > > the reason why the size of this category is not checked in loadlocale.c.
> > [2]
> >
> > Unless I'm wrong it may happen that a special case test for LC_CTYPE [3]
> > will no longer be necessary. I have no patch for this but the change would
> > be trivial, would you consider providing it yourself?
>
> it does seem like we should be able to drop that test. i'll put together
> a patch for it once it passes tests.
>
> btw, this code comes from this bug report:
> https://sourceware.org/bugzilla/show_bug.cgi?id=356
> and the analysis there seems to match your own.
> -mike