[Toybox] [PATCH] sort: Make -z give null-terminated output (like GNU)

[Rob Landley]

On 07/05/2015 12:08 AM, Kylie McClain wrote:
> This makes the `sort` implementation act like the GNU implementation,
> and changes -z to use \0 terminated input and output rather than just
> take \0 terminated input.

Sigh. I had a vague back burner idea of having -Z control output,
because having this not be orthogonal is creepy.

But that's the FSF for you. They created a broken API, and consistency
with that (in the absence of Posix advancing into the 21st century in
our lifetimes) trumps the interface not actually being stupid.

Not happy about it, but... modified version of this applied.

Rob

> ---
>  toys/posix/sort.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/toys/posix/sort.c b/toys/posix/sort.c
> index b7e16c9..ecbda0f 100644
> --- a/toys/posix/sort.c
> +++ b/toys/posix/sort.c
> @@ -33,7 +33,7 @@ config SORT_BIG
>      -M	month sort (jan, feb, etc).
>      -x	Hexadecimal numerical sort
>      -s	skip fallback sort (only sort with keys)
> -    -z	zero (null) terminated input
> +    -z	zero (null) terminated input/output

Just going with "lines" like the gnu/dammit man page does.

>      -k	sort by "key" (see below)
>      -t	use a key separator other than whitespace
>      -o	output to FILE instead of stdout
> @@ -386,7 +386,8 @@ void sort_main(void)
>      char *s = TT.lines[idx];
>      xwrite(fd, s, strlen(s));
>      if (CFG_TOYBOX_FREE) free(s);
> -    xwrite(fd, "\n", 1);
> +    if (CFG_SORT_BIG && (toys.optflags&FLAG_z)) xwrite(fd, "\0", 1);
> +    else xwrite(fd, "\n", 1);

We didn't #define FORCE_FLAGS before #including toys.h, so FLAG_z is
zero when configured out, so we don't need a separate test on
CFG_SORT_BIG because toys.optflags&FLAG_z becomes toys.optflags&0 which
becomes a constant 0 so the if (0) drops out. (That level of constant
propagation is guaranteed by c99 so you can use constant expressions to
initialize global variables outside function context. I had to know
this, and in fact fix it, when I maintained a tinycc fork last decade.)

That said, xwrite(fd, (toys.optfpags&FLAG_z) ? 0 : '\n', 1); avoids
duplicating the function pointer and two of its arguments.

Behind the scenes it probably turns into something like:
  int newtemp = 0, testtemp = toys.optflags & FLAG_z;
  conditional_assignment(newtemp, testtemp, '\n');
  xwrite(fd, newtemp, 1);

Because modern pipelined multi-core architectures (not SMP but multiple
instructions executed per clock in parallel) with register renaming and
stuff can do a calculation in parallel and choose not to save the result
afterwards (the second and third execution unit does it all the time)
without inserting bubbles in the pipeline, but branches are more of a
pain to deal with. So the "turning a branch into a conditional
assignment" thing is a simple optimization modern processors tend to
benefit from.

Of course what I'm really thinking is just that the ? : version is both
shorter and one line instead of two without being that harder to read,
and avoids repeating the function call and two of the arguments, without
actually having more tests. If the compiler _can't_ optimize better from
that starting point, it's a bad compiler. :)

But come to think of it, all that's wrong. There's a null terminator on
the string anyway, and since we're about to free it we can overwrite it
with \n if it should be a newline, and we're already calling strlen()
anyway so we might as well reduce the number of write syscalls we make...

(Sorry, late at night you get my thought process on trivial things like
this. See Pascal's Apology:
http://quoteinvestigator.com/2012/04/28/shorter-letter/)

Rob

 1436082160.0