[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#20029: 'yes' surprisingly slow
From: |
Pádraig Brady |
Subject: |
bug#20029: 'yes' surprisingly slow |
Date: |
Mon, 09 Mar 2015 22:14:57 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
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
- bug#20029: 'yes' surprisingly slow, Ole Tange, 2015/03/07
- bug#20029: 'yes' surprisingly slow, Pádraig Brady, 2015/03/07
- bug#20029: 'yes' surprisingly slow, Giuseppe Scrivano, 2015/03/09
- bug#20029: 'yes' surprisingly slow, Pádraig Brady, 2015/03/09
- bug#20029: 'yes' surprisingly slow, Ole Tange, 2015/03/10
- bug#20029: 'yes' surprisingly slow, Pádraig Brady, 2015/03/10