Re: enable -Werror for lib/ in coreutils

[Jim Meyering]

Paolo Bonzini wrote:

> On 10/29/2009 09:23 AM, Jim Meyering wrote:
>>> -           Idx idx = re_node_set_contains (dest_nodes, cur_node) - 1;
>>> > -           re_node_set_remove_at (dest_nodes, idx);
>>> > +           Idx idx = re_node_set_contains (dest_nodes, cur_node);
>>> > +           if (idx)
>>> > +             re_node_set_remove_at (dest_nodes, idx - 1);
>>
>> That looks like a good bug fix.
>
> Not really a bug fix---previously you'd pass -1 and that would be
> ignored; with your patch you pass SIZE_MAX and the effect is the same.

IMHO it is a bug fix.
A semantically unsigned variable must never be decremented to -1.
I didn't try to see if it could induce misbehavior.

>> Why use a signed type throughout rege*.[ch] when an unsigned one
>> more accurately models the data and interfaces?
>
> Because upstream uses a signed type, and I'm not sure we want to
> deviate from there.  I'd use intptr_t or ptrdiff_t.

We deviated years ago.
IMHO, reverting it would be contrary to the spirit of gnulib,
and is beyond the limited (warning avoidance) scope of my change.

Besides, we are not beholden to upstream.
For example, glibc could not change fts due to API and ABI constraints.
If I'd been content to stick with that implementation, numerous
limitations and outright bugs would remain, and we wouldn't have been
able to rely on it for the likes of chown, find, rm, du, etc.

My patches here are intended to induce no semantic change,
and to avoid the warnings about nonsense like "unsigned_var < 0".