Re: yes: rearrange code to prevent gcc10 warning

[Pádraig Brady]

On 30/01/2020 07:27, Christophe Meyering wrote:
> Hello,
>
> Using GCC10 on Fedora 31, I built coreutils from a git clone of the latest
> sources.
> 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
> [-Werror=stringop-overflow=]
>    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
> (bufalloc);
>        |                                                   ^~~~~~~~~~~~~~~~~~
>
> 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 
possible.
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?

thanks!
Pádraig

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;
   do
     {
       size_t operand_len = strlen (*operandp);