On 30/01/2020 18:25, Christophe Meyering wrote:
On Thu, Jan 30, 2020 at 5:05 AM Pádraig Brady <[email protected] <mailto:[email protected]>> 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?
> > That's even better, thank you! > Cool, I tweaked the summary to be prefixed with "build:" since this doesn't change the behavior of yes(1), and pushed. https://github.com/coreutils/coreutils/commit/dda53d7 thanks! Pádraig
