Jim Meyering wrote: > Pádraig Brady <[email protected]> wrote: >> Jim Meyering wrote: >>> Michael Meskes <[email protected]> wrote: >>>> On Wed, Dec 31, 2008 at 03:18:44PM +0100, Jim Meyering wrote: >>>>> Thanks, but "man statfs" on linux-based systems shows it can be signed: >>>>> ... >>>> Sorry for the noise, I just used grep -r to find stuff like >>>> >>>> /usr/include/asm-generic/statfs.h: __u32 f_files; >>>> /usr/include/asm-generic/statfs.h: __u64 f_files; >>>> /usr/include/asm-generic/statfs.h: __u64 f_files; >>> Have you seen ever stat print a negative number >>> corresponding to that field? Actually, I'll bet that >>> *has* happened... and considering the semantics >>> of that variable, I see no reason to print a signed value. >> On a related note, I was looking at removing -Wsign-compare warnings >> and noticed a lot are due to dealing with signed st_size, st_blocks etc. >> I searched and found no references to where these could ever be negative, >> and therefore I'm assuming that the type is only really to define the >> width and should always be interpreted as unsigned. >> >> So, I'll add an ST_SIZE() macro to system.h like: >> #define ST_SIZE(statbuf) ((uintmax_t) (statbuf).st_size) >> and also add the equivalent cast to ST_NBLOCKS etc. >> >> Then any code using them could deal just with unsigned variables, >> thus avoiding any -Wsign-compare warnings. > > Hi Pádraig, > > 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, 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. > 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. cheers, Pádraig. _______________________________________________ Bug-coreutils mailing list [email protected] http://lists.gnu.org/mailman/listinfo/bug-coreutils
