GNU bug report logs - #20029
'yes' surprisingly slow

Previous Next

Full log

Message #17 received at 20029 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Eric Blake <eblake <at> redhat.com>, Ole Tange <tange <at> gnu.org>, 
 20029 <at> debbugs.gnu.org
Subject: Re: bug#20029: 'yes' surprisingly slow
Date: Mon, 09 Mar 2015 22:14:57 +0000

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.