[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: yes: rearrange code to prevent gcc10 warning

From: Pádraig Brady
Subject: Re: yes: rearrange code to prevent gcc10 warning
Date: Thu, 30 Jan 2020 13:05:07 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:72.0) Gecko/20100101 Thunderbird/72.0

On 30/01/2020 07:27, Christophe Meyering wrote:

Using GCC10 on Fedora 31, I built coreutils from a git clone of the latest
After running
$./bootstrap && ./configure && make
I got a few deprecation warnings for sys/sysctl.h which I skipped over, and
found an interesting error while building src/yes.o:
   CC       src/yes.o
src/yes.c: In function 'main':
src/yes.c:110:20: error: writing 1 byte into a region of size 0
   110 |   buf[bufused - 1] = '\n';
       |   ~~~~~~~~~~~~~~~~~^~~~~~
src/yes.c:100:51: note: at offset -1 to an object with size 8192 allocated
by 'xmalloc' here
   100 |   char *buf = reuse_operand_strings ? *operands : xmalloc
       |                                                   ^~~~~~~~~~~~~~~~~~

The compiler didn't deduce that the for loop will always iterate at least
once, therefore my first thought was to assert(operands < operand_lim)
before the start of the for loop on line 102.
I 'heard' :) about assure and decided to use that instead, before realizing
that I could make it obvious that the loop would run at least once by
converting the for loop into a do-while loop.
This avoided the warning. I also made sure the tests still pass on F31.

Chris Meyering

Very nice.
Yes it's best to avoid warnings with standard language constructs where 
You've changed the loop structure from
"check", "run", "inc" to "run", "inc", "check",
which looks perfect.

I wonder would it be better to use the same construct in both loops.
Is it OK if I roll the following into your patch?


diff --git a/src/yes.c b/src/yes.c
index 335a23b..c357555 100644
--- a/src/yes.c
+++ b/src/yes.c
@@ -79,7 +79,8 @@ main (int argc, char **argv)
      large overhead of stdio buffering each item.  */
   size_t bufalloc = 0;
   bool reuse_operand_strings = true;
-  for (char **operandp = operands; operandp < operand_lim; operandp++)
+  char **operandp = operands;
+  do
       size_t operand_len = strlen (*operandp);
       bufalloc += operand_len + 1;
@@ -87,6 +88,7 @@ main (int argc, char **argv)
           && *operandp + operand_len + 1 != operandp[1])
         reuse_operand_strings = false;
+  while (++operandp < operand_lim);

   /* Improve performance by using a buffer size greater than BUFSIZ / 2.  */
   if (bufalloc <= BUFSIZ / 2)
@@ -99,7 +101,7 @@ main (int argc, char **argv)
      the operands strings; this wins when the buffer would be large.  */
   char *buf = reuse_operand_strings ? *operands : xmalloc (bufalloc);
   size_t bufused = 0;
-  char **operandp = operands;
+  operandp = operands;
       size_t operand_len = strlen (*operandp);

reply via email to

[Prev in Thread] Current Thread [Next in Thread]