On Mon, Nov 18, 2013 at 9:25 PM, Jim Meyering <jim@meyering.net> wrote:
> On Mon, Nov 18, 2013 at 6:16 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
>> Jim Meyering wrote:
>>>  static int
>>>  tstbit (unsigned int b, charclass const c)
>>>  {
>>> -  return c[b / INTBITS] & 1 << b % INTBITS;
>>> +  return c[b / INTBITS] & 1U << b % INTBITS;
>>>  }
>>
>> On a machine with 32-bit int and where b % INTBITS is 31,
>> the expression c[b / INTBITS] & 1U << b % INTBITS
>> is of type 'unsigned' and can have the value 2**31, and
>> this will overflow when tstbit converts that value as an int,
>> leading to implementation-defined behavior, which can include
>> raising a signal.
>>
>> Better would be something like this:
>>
>> static bool
>> tstbit (unsigned int b, charclass const c)
>> {
>>   return c[b / INTBITS] >> b % INTBITS & 1;
>> }
>>
>> and it'd probably be better to encourage this style in
>> other places where the problem occurs, e.g., quotearg.
>
> Good point.  "bool" is a better return type, too.
> I will adjust.

Here's an updated patch: