On Thu, Jan 30, 2020 at 5:05 AM Pádraig Brady <address@hidden
<mailto:address@hidden>> wrote:
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?