Re: bin/38004: /bin/sh truncates a message for unobvious reasons

[Robert Elz <kre%munnari.OZ.AU@localhost>]

The following reply was made to PR bin/38004; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: YAMAMOTO Takashi <yamt%mwd.biglobe.ne.jp@localhost>, David Laight <david%l8s.co.uk@localhost>,
        David Holland <dholland-bugs%netbsd.org@localhost>
Cc: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%netbsd.org@localhost
Subject: Re: bin/38004: /bin/sh truncates a message for unobvious reasons
Date: Sat, 01 Sep 2018 16:48:40 +0700

 This is the oldest remaining open /bin/sh related PR (that I know about
 anyway) so it is probably time to deal with it (I have skipped it up to now
 as it is really such a minor issue).
 
 David Holland <dholland-bugs%netbsd.org@localhost> wote (13 April 2008):
   | One of these days I'm intending to rework the parser, since it does
   | quite a lot of unclean things (see for example PRs 19832 and 35423)
   | and I'll fix this properly then.
 
 10 years have passed (and a bit more) and that reworking ie yet to
 happen ... the PRs mentioned (well, that PR, even though it had multiple
 numbers, that was just one issue) has been fixed, and the PRs closed.
 
 The parser is not really all that bad, and while it has had tweaks over
 the past few years, and may eventually get a few more, they are no more
 than tweaks, and do not amount to a "reworking" .... and I don't think we
 need that.
 
 But back to the substance of the PR:
 
 This was the proposed change (so that everyone does not
 need to go hunting in the PR or ancient nightmares...)
 
 +#if 1
 +	len = sizeof(ps->cmd);
 +#else
  	if (iflag || mflag || sizeof ps->cmd < 100)
  		len = sizeof(ps->cmd);
  	else
  		len = sizeof(ps->cmd) / 10;
 +#endif
 
 That is, when creating the jobs table, save as much of the entire string
 as fits in the space available, always, rather than just the first part.
 
  As was pointed out (more than a decade ago) not doing this provides a
 (tiny) saving when creating the jobs table entry for commands rn from a script,
 which almost never actually uses the value (having it at all is (almost) a
 waste of time).
 
 Also note that right now, sizeof ps->cmd is 200, so the "<100" tests
 achieve nothing (always false), and also means that sizeof()/10 == 20
 (and of that, 1 byte is used for the tailing \0, and if the command is
 longer, the last 3 bytes are replaced with an elipsis, to show trhat
 data was omitted, as shown in the example given ...
 
  [1]   Abort trap              /nfs/eos-fs.nfsk...
 
 there end up being just 16 meaningful characters of the command.
 (If the buffer size were reduced from 200 to 120, then just 8
 chars would be being saved for non-interactive uses.)
 
 But if we ignore the current value of MAXCMDTEXT (200) for now,
 from the code fragment above, it is clear that if MAXCMDTEXT were
 to be set to < 100, then the code (or the code's author) would be happy
 to save up to 99 (well 98, plus \0, -3 for the elipsis when needed) characters,
 regardless of whether it is running in a script or is interactive.
 
 So, it seems to me that a reasonable solution to this is to simply allow
 more chars to be saved in the non-interactive case, rather than the very
 small 19 byte limit, without necessarily allowing the full 199, which
 (even in cases like that shown) is likely to be more than are really needed.
 
 So, what I propose to do is to make that small algorithm just a fraction 
 smarter (mostly with compile time tests, so no extra executable code needs
 to run, just like the "sizeof() < 100" tests now) but with the result that we
 save more (say a miniumm of up to about 50 chars) and have what is saved
 still depend upon MAXCMDTEXT (which is the same as the sizeof()) but
 in a slightly less obnoxious/brutal way than it is now.
 
 Does anyone have any issues with that (before I actually make the changes)?
 Obviously after it has changed, there will be more opportunities for
 comments and more adjustments can be made if needed.
 
 I really do not believe that mass changes to the way that things work
 currently is needed in order to provide a solution here that will be good
 enough for everyone...
 
 kre