Pádraig Brady wrote: > On 28/05/11 22:47, Jim Meyering wrote: >> Jim Meyering wrote: >> >>> These (off_t) casts are anachronistic. >>> They were useful in pre-ANSI-C days, i.e., before prototypes. >>> There are two remaining off_t casts, and neither appears useful: >>> (one is even inconsistently formatted, with no space after the ")" ;-) >>> >>> src/shred.c: if (offset > OFF_T_MAX - (off_t) soff) >>> src/truncate.c: if (ssize > OFF_T_MAX - (off_t)fsize) >>> >>> So I'll probably remove them, too. >> >> Now I'm not so sure. >> soff is of type size_t and fsize is uintmax_t, >> each of which may be wider than off_t. >> I suspect that each of these trigger one of the warnings >> that we have not enabled. > > Yes it will trigger -Wsign-compare which has helped > me find hard to spot bugs. It will also cause a bug I > think when ssize is negative, as then it will be promoted > to a large positive number for the comparison. > The truncate-overflow test catches this change in behavior.
Hah! It was definitely too late. For me to write the above without even running "make check"... Patch discarded. Thanks. I was tempted to revisit enabling -Wsign-compare for src/, but count over 50 warnings/errors, mostly for code where a "fix" would involve adding casts. So I still think -Wsign-compare is not worth it. However, there's at least one change that is clean and that does address one of those warnings: >From 0d0b2c3108bf85c3d71ca9dc688898e715354cff Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Sun, 29 May 2011 11:28:29 +0200 Subject: [PATCH] maint: placate -Wsign-compare when it's non-invasive * src/stdbuf.c: Declare loop index to be unsigned. --- src/stdbuf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/stdbuf.c b/src/stdbuf.c index 607859c..2f66dd5 100644 --- a/src/stdbuf.c +++ b/src/stdbuf.c @@ -257,7 +257,7 @@ set_LD_PRELOAD (void) static void set_libstdbuf_options (void) { - int i; + unsigned int i; for (i = 0; i < ARRAY_CARDINALITY (stdbuf); i++) { -- 1.7.5.2.660.g9f46c
