Re: stat signed/unsigned

[Jim Meyering]

Pádraig Brady <address@hidden> wrote:
...
>> That sounds like it could be rather invasive...
>>>From an aesthetics/readability point of view, I'm not sure
>> I like the idea of using ST_SIZE (st) in place of "st.st_size".
>
> Well it would be more consistent as we already use ST_BLKSIZE etc.
> There aren't many references to .st_size really.
>
>> More importantly, there are places in the code that compare stat.st_size
>> against negative numbers (at least remove.c).
>
> Yuk, so that code assumes that st_size will be always be signed,

It's guaranteed that st_size's type is signed, since off_t is signed.

> and writes negative numbers in to flag various things.
> remove.c is already -Wsign-compare clean so I can just not
> use ST_SIZE() there.

Using a macro to cast away such "known" signedness sounds
dangerous.

>> I've resisted making coreutils "-Wsign-compare"-warning free for years,
>> because the cost of the required changes seems too high.
>> This might be one of those cases where it's better to mark
>> known-false-positive warnings and automatically filter them out
>> of a separate compilation with just -Wsign-compare.
>
> That's a good suggestion but nearly as much work as fixing up the issues.
>
>> I've started down this clean-up-Wsign-compare-warnings road
>> a few times, and inevitably end up concluding it's not worthwhile.
>
> The current patch I have is 50 insertions(+), 37 deletions(-)
> So for 13 new lines of code with no new casts I think it
> probably is worth addressing.

show the code ;-)