Re: [hackers] [sbase][PATCH] Minor optimizations for 'yes'

From: Michael Forney <mforney_AT_mforney.org>
Date: Fri, 28 Jun 2019 01:06:56 -0700

On 2019-06-27, Laslo Hunhold <dev_AT_frign.de> wrote:
> On Thu, 27 Jun 2019 09:10:28 +0200
> Quentin Rameau <quinq_AT_fifth.space> wrote:
>
> Dear Quentin,
>
>> I agree, yes is neither a benchmark tool nor a data generator, it just
>> outputs 'y' for piping into other commands.
>> Keep it simple.
>
> I agree with you in general, but are we really "optimizing" yes(1) here
> for the sake of performance? This looks to me like a case of premature
> optimization.

I think we are all on the same page here about simplifying yes(1)
rather than optimizing it. See aidanwillie0317's most recent patch.

> We don't know if a script relies on the behaviour of yes(1) printing
> all passed strings repeatedly. So, even though the tool is not
> standardized by Posix the common "consensus" is that
>
> - with no arguments, it shall print y and a newline repeatedly.
> - with arguments, it shall print them, comma separated, followed by
> newline, repeatedly.

(I think you mean "space separated" here)

How did you determine this consensus? All the BSD implementations I
looked at (OpenBSD, FreeBSD, NetBSD) only support a single argument.

Do you have any examples of scripts relying on this behavior of yes(1)?

> The point about readability is a good one. I will even take the blame
> for writing it as it is now. However, you could greatly improve
> readability without sacrificing features/performance. The line
>
> argv0 = *argv, argv0 ? (argc--, argv++) : (void *)0;
>
> is used in many places in sbase and is more or less an idiom. The
> entire tool itself could then be written as follows. We can even
> remove the checks consequently within the loop, which could be regarded
> as a "performance" improvement.
>
> int i;
>
> argv0 = *argv, argv0 ? (argc--, argv++) : (void *)0;
>
> if (argc == 0) {
> for (;;) {
> fputs("y\n", stdout);
> }
> } else {
> for (;;) {
> for (i = 0; i < argc; i++) {
> fputs(argv[i], stdout);
> fputc(' ', stdout);

This will end up printing an extra space before the newline.

> }
> fputc('\n', stdout);
> }
> }
>
> What do you think? Patch is attached.

This looks pretty good to me. If we decide to support any number of
operands, this seems like the way to do it.
Received on Fri Jun 28 2019 - 10:06:56 CEST