CVS commit: src/bin/sh

["Robert Elz" <kre%netbsd.org@localhost>]

Module Name:    src
Committed By:   kre
Date:           Sun Nov 18 17:23:37 UTC 2018

Modified Files:
        src/bin/sh: expand.c expand.h parser.c syntax.h

Log Message:
Rationalise (slightly) the way that expansions are processed
to hide meta-characters in the result when the expansion was
in (double) quotes, and so should not be further processed.

Most of this has been OK for a long while, but \ needs hiding
as well, which complicates things, as \ cannot simply be hidden
in the syntax tables as one of the group of random special characters.

This was fixed earlier for simple variable expansions, but
every variety has its own code path ($var uses different code
than $n which is different than $(...), which is different
again from ~ expansions, and also from what $'...' produces).

This could be fixed by moving them all to a common code path,
but that's harder than it seems.  The form in which the data
is made available differs, so one common routine would need
a whole bunch of different "get the next char or indicate end"
methods - probably via passing in an accessor function.
That's all a lot of churn, and would probably slow the shell.

Instead, just make macros for doing the standard tests, and
use those instead of open coding (differently) each time.
This way some of the code paths don't end up forgetting to
handle '\' (which is different than all the others).

This removes one optimisation ... when no escaping is needed
(like just $var (unquoted) where magic chars (think '*') in
the value are intended to remain magic), the code avoided doing
two tests for each char ("do we need escapes" and "is this char
one that needs escaping") by choosing two different syntax
tables (choice made outside the loop) - one of which never
returns the magic "needs escaping" result, and the other does
when appropriate, and then just avoiding the "do we need escapes"
test for each character processed.   Then when '\' was fixed,
there needed to be another test for it, as it cannot (for other
reasons) be the same as all the others for which "this char
need escaping" is true.   So that added a 2nd test for each char...
Not all the code paths were updated.   Hence the bugs...

nb: this is all rarely seen in the wild, so it is no big
surprised that no-one ever noticed.

Now the "use two different syntax tables" is gone (the two
returned the same for '\' which is why '\' needed special
processing) - and in order to avoid two tests for each
char (plus the \ test) we duplicate the loops, one of which
tests each char to see if it needs an escape, the 2nd just
copies them.   This should be faster in the "no escapes"
code path (though that is not the point) and perhaps also
in the "escapes needed" path (no indirect reference to
the syntax table - though that would probably be in a
register) but makes the code slightly bigger.  For /bin/sh
the text segment (on amd64) has grown by 48 bytes.  But
it still uses the same number of 512 byte pages (and hence
also any bigger page size).  The resulting file size
(/bin/sh) is identical before and after.  So is /rescue/sh
(or /rescue/anything-else).


To generate a diff of this commit:
cvs rdiff -u -r1.127 -r1.128 src/bin/sh/expand.c
cvs rdiff -u -r1.23 -r1.24 src/bin/sh/expand.h
cvs rdiff -u -r1.152 -r1.153 src/bin/sh/parser.c
cvs rdiff -u -r1.9 -r1.10 src/bin/sh/syntax.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.