Pádraig Brady <[EMAIL PROTECTED]> wrote:
> Rather than relying on code inspection to find places
> where signed ints would be erroneously promoted to unsigned,
> I went looking for a gcc option to automatically detect this.
> It's -Wsign-compare which is automatically included in -Wextra.
> Note it might be good to add to README-hacking to run configure
> with the CFLAGS="-Wall -Wextra" options?
>
> So turning that on didn't show any more bugs, but I've attached
> a patch to fix the warnings in a couple of places where the
> code was ambiguous.
Thanks. Applied.
Note however that some of the -W* options (possibly including
that one) induce warnings for which the least-bad solution is
to add a cast that results in less-maintainable code. So while
I'm very fond of avoiding warnings, I choose carefully which -W
options to use.
> There is still a warning given by the ST_BLKSIZE() macro in
> src/system.h where it compares a blksize_t which is long int
> on my system, to SIZE_MAX. Perhaps a cast to (size_t) is
> required there to confirm the intent?
Sure. Sounds reasonable, and looks safe.
A patch would be welcome.
BTW, I've just fixed a couple easy/safe ones:
(the base64/feof one was technically a legitimate warning)
>From 5cc42f7de6251792a02befab4b3df7b3023135d8 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[EMAIL PROTECTED]>
Date: Fri, 27 Jun 2008 16:26:05 +0200
Subject: [PATCH] base64: don't rely on feof returning 0/1
* src/base64.c (do_decode): feof is specified to return nonzero,
not 0/1, so use "k < 1 + !!feof(in)" as the loop termination test.
---
src/base64.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/base64.c b/src/base64.c
index 5067b28..416e573 100644
--- a/src/base64.c
+++ b/src/base64.c
@@ -214,7 +214,7 @@ do_decode (FILE *in, FILE *out, bool ignore_garbage)
However, when it processes the final input buffer, we want
to iterate it one additional time, but with an indicator
telling it to flush what is in CTX. */
- for (k = 0; k < 1 + feof (in); k++)
+ for (k = 0; k < 1 + !!feof (in); k++)
{
if (k == 1 && ctx.i == 0)
break;
--
1.5.6.66.g30faa
>From 5b610a06b297b162f3c63e139e33eb0ca9113f70 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[EMAIL PROTECTED]>
Date: Fri, 27 Jun 2008 16:34:00 +0200
Subject: [PATCH] avoid a -Wsign-compare warning
* src/tee.c (tee_files): Swap fwrite's size/n_elem args and
compare the return value against "1".
---
src/tee.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/tee.c b/src/tee.c
index 162455c..4e46aab 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -190,7 +190,7 @@ tee_files (int nfiles, const char **files)
Standard output is the first one. */
for (i = 0; i <= nfiles; i++)
if (descriptors[i]
- && fwrite (buffer, 1, bytes_read, descriptors[i]) != bytes_read)
+ && fwrite (buffer, bytes_read, 1, descriptors[i]) != 1)
{
error (0, errno, "%s", files[i]);
descriptors[i] = NULL;
--
1.5.6.66.g30faa
_______________________________________________
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
