NetBSD Problem Report #38004
From yamt@mwd.biglobe.ne.jp Tue Feb 12 08:16:08 2008
Return-Path: <yamt@mwd.biglobe.ne.jp>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
by narn.NetBSD.org (Postfix) with ESMTP id 3B1F863BD1C
for <gnats-bugs@gnats.NetBSD.org>; Tue, 12 Feb 2008 08:16:08 +0000 (UTC)
Message-Id: <20080212081609.4A71911703@yamt.dyndns.org>
Date: Tue, 12 Feb 2008 17:16:09 +0900 (JST)
From: yamt@mwd.biglobe.ne.jp
Reply-To: yamt@NetBSD.org
To: gnats-bugs@gnats.NetBSD.org
Subject: /bin/sh truncates a message for unobvious reasons
X-Send-Pr-Version: 3.95
>Number: 38004
>Category: bin
>Synopsis: /bin/sh truncates a message for unobvious reasons
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: bin-bug-people
>State: closed
>Class: change-request
>Submitter-Id: net
>Arrival-Date: Tue Feb 12 08:20:02 +0000 2008
>Closed-Date: Tue Jan 01 18:49:08 +0000 2019
>Last-Modified: Tue Jan 01 18:49:08 +0000 2019
>Originator: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
>Release: NetBSD 4.99.54
>Organization:
>Environment:
>Description:
when printing ps->cmd, commandtext() truncates it for
unobvious reasons.
>How-To-Repeat:
>Fix:
Index: jobs.c
===================================================================
RCS file: /cvsroot/src/bin/sh/jobs.c,v
retrieving revision 1.65
diff -u -p -r1.65 jobs.c
--- jobs.c 24 Apr 2006 19:00:29 -0000 1.65
+++ jobs.c 12 Feb 2008 08:06:25 -0000
@@ -1209,10 +1209,14 @@ commandtext(struct procstat *ps, union n
int len;
cmdnextc = ps->cmd;
+#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
cmdnleft = len;
cmdtxt(n);
if (cmdnleft <= 0) {
>Release-Note:
>Audit-Trail:
From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: bin/38004: /bin/sh truncates a message for unobvious reasons
Date: Tue, 12 Feb 2008 18:33:04 +0000
On Tue, Feb 12, 2008 at 08:20:02AM +0000, yamt@mwd.biglobe.ne.jp wrote:
> >Number: 38004
> >Synopsis: /bin/sh truncates a message for unobvious reasons
>
> >Environment:
> >Description:
> when printing ps->cmd, commandtext() truncates it for
> unobvious reasons.
> >How-To-Repeat:
>
> >Fix:
> +#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
When is this a problem?
IIRC the purpose of the test is to speed up shell scripts - where it
is really pointless regenerating the command line in this form at all.
David
--
David Laight: david@l8s.co.uk
From: yamt@mwd.biglobe.ne.jp (YAMAMOTO Takashi)
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: bin/38004: /bin/sh truncates a message for unobvious reasons
Date: Wed, 13 Feb 2008 19:18:09 +0900 (JST)
> The following reply was made to PR bin/38004; it has been noted by GNATS.
>
> From: David Laight <david@l8s.co.uk>
> To: gnats-bugs@NetBSD.org
> Cc:
> Subject: Re: bin/38004: /bin/sh truncates a message for unobvious reasons
> Date: Tue, 12 Feb 2008 18:33:04 +0000
>
> On Tue, Feb 12, 2008 at 08:20:02AM +0000, yamt@mwd.biglobe.ne.jp wrote:
> > >Number: 38004
> > >Synopsis: /bin/sh truncates a message for unobvious reasons
> >
> > >Environment:
> > >Description:
> > when printing ps->cmd, commandtext() truncates it for
> > unobvious reasons.
> > >How-To-Repeat:
> >
> > >Fix:
> > +#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
>
> When is this a problem?
> IIRC the purpose of the test is to speed up shell scripts - where it
> is really pointless regenerating the command line in this form at all.
>
> David
>
> --
> David Laight: david@l8s.co.uk
outputs like the following is not very useful.
YAMAMOTO Takashi
obj ===> games/backgammon
obj ===> bin/dd
obj ===> gnu/lib/crtstuff4
obj ===> games/banner
[1] Abort trap /nfs/eos-fs.nfsk...
--- obj-backgammon ---
*** [obj-backgammon] Error code 134
A failure has been detected in another branch of the parallel make
nbmake: stopped in /siro/nbsd/src/bin/chio
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, yamt@mwd.biglobe.ne.jp
Subject: Re: bin/38004: /bin/sh truncates a message for unobvious reasons
Date: Sun, 13 Apr 2008 03:03:59 +0000
On Tue, Feb 12, 2008 at 06:35:01PM +0000, David Laight wrote:
> > >Fix:
> > +#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
>
> When is this a problem?
> IIRC the purpose of the test is to speed up shell scripts - where it
> is really pointless regenerating the command line in this form at all.
Shouldn't the proper fix for that to be to either save a copy of the
original command line string, or to only produce the string when it's
needed? Or am I missing something obvious?
--
David A. Holland
dholland@netbsd.org
From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: bin/38004: /bin/sh truncates a message for unobvious reasons
Date: Sun, 13 Apr 2008 21:30:53 +0100
On Sun, Apr 13, 2008 at 03:05:02AM +0000, David Holland wrote:
> The following reply was made to PR bin/38004; it has been noted by GNATS.
>
> From: David Holland <dholland-bugs@netbsd.org>
> To: gnats-bugs@NetBSD.org
> Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, yamt@mwd.biglobe.ne.jp
> Subject: Re: bin/38004: /bin/sh truncates a message for unobvious reasons
> Date: Sun, 13 Apr 2008 03:03:59 +0000
>
> On Tue, Feb 12, 2008 at 06:35:01PM +0000, David Laight wrote:
> > > >Fix:
> > > +#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
> >
> > When is this a problem?
> > IIRC the purpose of the test is to speed up shell scripts - where it
> > is really pointless regenerating the command line in this form at all.
>
> Shouldn't the proper fix for that to be to either save a copy of the
> original command line string, or to only produce the string when it's
> needed? Or am I missing something obvious?
It needs the command string after substitutions, this only exits as a
tree, and the tree nodes are allocated from a 'stack allocator' and
are all thrown away (by resetting the base) once the comand is started.
Thus you have to walk the tree nodes to generate a printable string.
(I'm sure Mr Bourne's shell didn't use this sort of parser!)
David
--
David Laight: david@l8s.co.uk
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, yamt@mwd.biglobe.ne.jp
Subject: Re: bin/38004: /bin/sh truncates a message for unobvious reasons
Date: Sun, 13 Apr 2008 21:34:14 +0000
On Sun, Apr 13, 2008 at 08:35:04PM +0000, David Laight wrote:
>>> IIRC the purpose of the test is to speed up shell scripts - where it
>>> is really pointless regenerating the command line in this form at all.
>>
>> Shouldn't the proper fix for that to be to either save a copy of the
>> original command line string, or to only produce the string when it's
>> needed? Or am I missing something obvious?
>
> It needs the command string after substitutions, this only exits as a
> tree, and the tree nodes are allocated from a 'stack allocator' and
> are all thrown away (by resetting the base) once the comand is started.
> Thus you have to walk the tree nodes to generate a printable string.
Ah. Well, probably it ought to keep the tree around, or at least
enough of it to generate the string.
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.
In the meantime, I think a better hack for speeding up scripts would
be to traverse the tree only to the first word and then stop.
> (I'm sure Mr Bourne's shell didn't use this sort of parser!)
You think? :-)
--
David A. Holland
dholland@netbsd.org
From: Robert Elz <kre@munnari.OZ.AU>
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>, David Laight <david@l8s.co.uk>,
David Holland <dholland-bugs@netbsd.org>
Cc: gnats-bugs@NetBSD.org, netbsd-bugs@netbsd.org
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> 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
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: bin/38004: /bin/sh truncates a message for unobvious reasons
Date: Sat, 1 Sep 2018 23:00:53 +0000
On Sat, Sep 01, 2018 at 09:55:01AM +0000, Robert Elz wrote:
> 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).
Right...
> | 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.
Yeah, that's what my todo list is like. :-(
My conclusion when I looked at the parser then was that I should write
a new one; but it was doing fairly regrettable things at the time and
I have high standards for parsers.
It does not have the symptoms now that it did then, in any case, so
while I might still do that sometime it's no longer even half a
priority.
> 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.
That seems fine.
--
David A. Holland
dholland@netbsd.org
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: bin/38004: /bin/sh truncates a message for unobvious reasons
Date: Sun, 02 Sep 2018 06:48:16 +0700
Date: Sat, 1 Sep 2018 23:05:00 +0000 (UTC)
From: David Holland <dholland-bugs@netbsd.org>
Message-ID: <20180901230500.E933A7A1FA@mollari.NetBSD.org>
| > So, what I propose to do is to make that small algorithm just a
| > fraction smarter
| That seems fine.
What I tentatively have (in uncommitted, mostly untested so far) sources is ...
--- jobs.c 30 Dec 2017 23:24:19 -0000 1.98
+++ jobs.c 1 Sep 2018 23:36:32 -0000
@@ -1509,8 +1509,12 @@
int len;
cmdnextc = ps->cmd;
- if (iflag || mflag || sizeof ps->cmd < 100)
+ if (iflag || mflag || sizeof ps->cmd <= 60)
len = sizeof(ps->cmd);
+ else if (sizeof ps->cmd <= 400)
+ len = 50;
+ else if (sizeof ps->cmd <= 800)
+ len = 80;
else
len = sizeof(ps->cmd) / 10;
cmdnleft = len;
which after all the constant tests are removed, with current settings,
turns into
if (iflag || mflag)
len = sizeof(ps->cmd); /* 200 */
else
len = 50;
But it will scale upwards a bit if a decision is ever made to make the command
string buffer bigger (or if someone ever does that for private use.)
50 turns out to be a nice size to make the output (when printed, along with
other noise that appears) fit in 80 columns, and changes ...
[1] + 12256 Running /tmp/mycmd CVS F...
into
[1] + 18902 Running /tmp/mycmd CVS FIX FIX2 MSG MSG.bak Makefile T...
(/tmp/mycmd is simply a script that does sleep for a few seconds, and ignores
its args, allows testing of long command lines without needing to find
something which core dumps.)
If this seems reasonable to everyone (or at least those concerned) I will
commit this in head (I kind of doubt this is really worth pulling up to -8,
especially after we have lived with it for all these years, but that could
happen too if requested.)
kre
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: bin/38004: /bin/sh truncates a message for unobvious reasons
Date: Tue, 04 Sep 2018 04:45:24 +0700
Since the most recent buildbot evbppc build encountered ...
Making lj4_font.n from /home/source/ab/HEAD/src/external/gpl2/groff/dist/src/devices/grolj4/lj4_font.man
[1] Segmentation fault ${runcmd} "${mak...
which is close to useless as a message, because of exactly this
issue, I am going to commit the changes I sent earlier (as no-one made
any comment about those - they can still be revised.)
Not that this will directly help the buildbots right now, as they
don't run -current and won't get the change anytime soon I
suspect, but sometime in the future, this might help someone.
kre
ps: commit will happen after my test build succeeds, and I get a chance
to make sure it is all working as it should in a real build.
From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/38004 CVS commit: src/bin/sh
Date: Tue, 4 Sep 2018 01:09:29 +0000
Module Name: src
Committed By: kre
Date: Tue Sep 4 01:09:29 UTC 2018
Modified Files:
src/bin/sh: jobs.c
Log Message:
PR bin/38004
Save more characters of command in non-interactive jobs, in case of
core dumps and similar (16 effective chars was a few too little).
Arrange for number to increase if command buffer size increases.
To generate a diff of this commit:
cvs rdiff -u -r1.98 -r1.99 src/bin/sh/jobs.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: PR/38004 CVS commit: src/bin/sh
Date: Fri, 28 Dec 2018 21:01:08 +0000
On Tue, Sep 04, 2018 at 01:10:00AM +0000, Robert Elz wrote:
> Log Message:
> PR bin/38004
>
> Save more characters of command in non-interactive jobs, in case of
> core dumps and similar (16 effective chars was a few too little).
>
> Arrange for number to increase if command buffer size increases.
Should we close this?
--
David A. Holland
dholland@netbsd.org
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: PR/38004 CVS commit: src/bin/sh
Date: Sat, 29 Dec 2018 08:23:28 +0700
Date: Fri, 28 Dec 2018 21:05:01 +0000 (UTC)
From: David Holland <dholland-bugs@netbsd.org>
Message-ID: <20181228210501.6C84C7A21F@mollari.NetBSD.org>
| Should we close this?
No objections from me (obviously) - and if what we have now isn't
good enough, this could be re-opened, or another PR created.
kre
State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Tue, 01 Jan 2019 18:49:08 +0000
State-Changed-Why:
The behavior's been improved; there doesn't seem to be any collective will
to make broader changes. If you're still unhappy feel free to reoopen...
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.43 2018/01/16 07:36:43 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home