Re: [PATCH] maint: remove useless (off_t) cast of lseek arg

[Jim Meyering]

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 <address@hidden>
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