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

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

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