This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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

Attachment: signature.asc
Description: Digital signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]