On 09/03/15 20:02, Eric Blake wrote:
> On 03/09/2015 01:47 PM, Pádraig Brady wrote:
>
>>
>> Note this change also ensures that yes will only write complete lines
>> for lines softer than BUFSIZ.
>
> s/softer/smaller/
>
>>
>> * src/yes.c (main): Build up a BUFSIZ buffer of lines,
>> and output that, rather than having stdio process each item.
>> * tests/misc/yes.sh: Add a new test for various buffer sizes.
>> * tests/local.mk: Reference the new test.
>> Fixes http://bugs.gnu.org/20029
>> ---
>> src/yes.c | 43 +++++++++++++++++++++++++++++++++++++++++--
>> tests/local.mk | 1 +
>> tests/misc/yes.sh | 28 ++++++++++++++++++++++++++++
>> 3 files changed, 70 insertions(+), 2 deletions(-)
>> create mode 100755 tests/misc/yes.sh
>>
>> diff --git a/src/yes.c b/src/yes.c
>> index b35b13f..91dea11 100644
>> --- a/src/yes.c
>> +++ b/src/yes.c
>> @@ -58,6 +58,10 @@ Repeatedly output a line with all specified STRING(s), or
>> 'y'.\n\
>> int
>> main (int argc, char **argv)
>> {
>> + char buf[BUFSIZ];
>
> Do you really want this stack-allocated? BUFSIZ can be larger than a
> page, which can then interfere with stack overflow detection.
Well we do such stack buffers elsewhere:
$ git grep char.*\\[BUFSIZ\\]
src/head.c: char buf[BUFSIZ];
src/head.c: char buffer[BUFSIZ];
src/head.c: char buffer[BUFSIZ];
src/head.c: char buffer[BUFSIZ];
src/head.c: char buffer[BUFSIZ];
src/ls.c: char smallbuf[BUFSIZ];
src/od.c: char buf[BUFSIZ];
src/tail.c: char buffer[BUFSIZ];
src/tail.c: char buffer[BUFSIZ];
src/tail.c: char buffer[BUFSIZ];
src/tail.c: char buffer[BUFSIZ];
src/tail.c: char buffer[BUFSIZ];
src/tail.c: char buffer[BUFSIZ];
src/tee.c: char buffer[BUFSIZ];
src/tr.c:static char io_buf[BUFSIZ];
src/uptime.c: char buf[BUFSIZ];
src/yes.c: char buf[BUFSIZ];
We would probably change them all if this was thought to be a problem.
>> +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
>> +print_ver_ yes
>> +
>> +for size in 1 4095 4096 8191 8192 16383 16384; do
>
> Should you also test 4097 8193 and 16385 (that is, a likely
> 1-more-than-BUFSIZ in the mix)?
The 1 more is implicit with the \n added by yes(1).
thanks for the review!
Pádraig
