Re: truncate.c fails to compile on make distcheck

[Jim Meyering]

Pádraig Brady <address@hidden> 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 <address@hidden>
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 <address@hidden>
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