Re: CVS commit: src/bin/sh

2020-02-06 Thread Robert Elz
Date:Fri, 7 Feb 2020 01:25:08 +
From:"Santhosh Raju" 
Message-ID:  <20200207012508.38c9cf...@cvs.netbsd.org>

  | bin/sh: Fixes -Werror=shadow causing build breaks.

Thanks for that.

kre



CVS commit: src/bin/sh

2019-10-14 Thread Christos Zoulas
Module Name:src
Committed By:   christos
Date:   Mon Oct 14 13:34:14 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
remove masking and cast (requested by kre@)


To generate a diff of this commit:
cvs rdiff -u -r1.135 -r1.136 src/bin/sh/expand.c

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



CVS commit: src/bin/sh

2019-10-14 Thread Christos Zoulas
Module Name:src
Committed By:   christos
Date:   Mon Oct 14 13:34:14 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
remove masking and cast (requested by kre@)


To generate a diff of this commit:
cvs rdiff -u -r1.135 -r1.136 src/bin/sh/expand.c

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

Modified files:

Index: src/bin/sh/expand.c
diff -u src/bin/sh/expand.c:1.135 src/bin/sh/expand.c:1.136
--- src/bin/sh/expand.c:1.135	Sun Oct 13 16:55:04 2019
+++ src/bin/sh/expand.c	Mon Oct 14 09:34:14 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: expand.c,v 1.135 2019/10/13 20:55:04 christos Exp $	*/
+/*	$NetBSD: expand.c,v 1.136 2019/10/14 13:34:14 christos Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)expand.c	8.5 (Berkeley) 5/15/95";
 #else
-__RCSID("$NetBSD: expand.c,v 1.135 2019/10/13 20:55:04 christos Exp $");
+__RCSID("$NetBSD: expand.c,v 1.136 2019/10/14 13:34:14 christos Exp $");
 #endif
 #endif /* not lint */
 
@@ -268,8 +268,7 @@ argstr(const char *p, int flag)
 			return p - 1;
 		case CTLENDVAR: /* end of expanding yyy in ${xxx-yyy} */
 		case CTLENDARI: /* end of a $(( )) string */
-			if (had_dol_at &&
-			(*p & 0xff) == (unsigned char)CTLQUOTEEND)
+			if (had_dol_at && *p == CTLQUOTEEND)
 p++;
 			NULLTERM_4_TRACE(expdest);
 			VTRACE(DBG_EXPAND, ("argstr returning at \"%.6s\"..."



CVS commit: src/bin/sh

2019-10-13 Thread Christos Zoulas
Module Name:src
Committed By:   christos
Date:   Sun Oct 13 20:55:04 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
prevent sign extension from making expression always false.


To generate a diff of this commit:
cvs rdiff -u -r1.134 -r1.135 src/bin/sh/expand.c

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



CVS commit: src/bin/sh

2019-10-13 Thread Christos Zoulas
Module Name:src
Committed By:   christos
Date:   Sun Oct 13 20:55:04 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
prevent sign extension from making expression always false.


To generate a diff of this commit:
cvs rdiff -u -r1.134 -r1.135 src/bin/sh/expand.c

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

Modified files:

Index: src/bin/sh/expand.c
diff -u src/bin/sh/expand.c:1.134 src/bin/sh/expand.c:1.135
--- src/bin/sh/expand.c:1.134	Mon Oct  7 23:53:57 2019
+++ src/bin/sh/expand.c	Sun Oct 13 16:55:04 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: expand.c,v 1.134 2019/10/08 03:53:57 kre Exp $	*/
+/*	$NetBSD: expand.c,v 1.135 2019/10/13 20:55:04 christos Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)expand.c	8.5 (Berkeley) 5/15/95";
 #else
-__RCSID("$NetBSD: expand.c,v 1.134 2019/10/08 03:53:57 kre Exp $");
+__RCSID("$NetBSD: expand.c,v 1.135 2019/10/13 20:55:04 christos Exp $");
 #endif
 #endif /* not lint */
 
@@ -268,12 +268,13 @@ argstr(const char *p, int flag)
 			return p - 1;
 		case CTLENDVAR: /* end of expanding yyy in ${xxx-yyy} */
 		case CTLENDARI: /* end of a $(( )) string */
-			if (had_dol_at && (*p&0xFF) == CTLQUOTEEND)
+			if (had_dol_at &&
+			(*p & 0xff) == (unsigned char)CTLQUOTEEND)
 p++;
 			NULLTERM_4_TRACE(expdest);
 			VTRACE(DBG_EXPAND, ("argstr returning at \"%.6s\"..."
 			" after %2.2X; added \"%s\" to expdest\n",
-			p, (c&0xff), stackblock()));
+			p, (c & 0xff), stackblock()));
 			return p;
 		case CTLQUOTEMARK:
 			/* "$@" syntax adherence hack */



CVS commit: src/bin/sh

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 03:53:57 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
Remove a (completely harmless) duplicate assignment introduced in a
code merge from FreeBSD in 2017.   NFC.

Pointed out by Roland Illig.


To generate a diff of this commit:
cvs rdiff -u -r1.133 -r1.134 src/bin/sh/expand.c

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

Modified files:

Index: src/bin/sh/expand.c
diff -u src/bin/sh/expand.c:1.133 src/bin/sh/expand.c:1.134
--- src/bin/sh/expand.c:1.133	Tue Oct  8 03:52:44 2019
+++ src/bin/sh/expand.c	Tue Oct  8 03:53:57 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: expand.c,v 1.133 2019/10/08 03:52:44 kre Exp $	*/
+/*	$NetBSD: expand.c,v 1.134 2019/10/08 03:53:57 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)expand.c	8.5 (Berkeley) 5/15/95";
 #else
-__RCSID("$NetBSD: expand.c,v 1.133 2019/10/08 03:52:44 kre Exp $");
+__RCSID("$NetBSD: expand.c,v 1.134 2019/10/08 03:53:57 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -1955,7 +1955,6 @@ patmatch(const char *pattern, const char
 			}
 			/* end shortcut */
 
-			invert = 0;
 			savep = p, saveq = q;
 			invert = 0;
 			if (*p == '!' || *p == '^') {



CVS commit: src/bin/sh

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 03:53:57 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
Remove a (completely harmless) duplicate assignment introduced in a
code merge from FreeBSD in 2017.   NFC.

Pointed out by Roland Illig.


To generate a diff of this commit:
cvs rdiff -u -r1.133 -r1.134 src/bin/sh/expand.c

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



CVS commit: src/bin/sh

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 03:52:44 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
Open code the validity test & copy of the character class name in
a bracket expression in a pattern (ie: [[:THISNAME:]]).   Previously
the code used strspn() to look for invalid chars in the name, and
then memcpy(), now we do the test and copy a character at a time.
This might, or might not, be faster, but it now correctly handles
\ quoted characters in the name (' and " quoting were already
dealt with, \ was too in an earlier version, but when the \ handling
changes were made, this piece of code broke).

Not exactly a vital bug fix (who writes [[:\alpha:]] or similar?)
but it should work correctly regardless of how obscure the usage is.

Problem noted by Harald van Dijk

XXX pullup -9


To generate a diff of this commit:
cvs rdiff -u -r1.132 -r1.133 src/bin/sh/expand.c

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

Modified files:

Index: src/bin/sh/expand.c
diff -u src/bin/sh/expand.c:1.132 src/bin/sh/expand.c:1.133
--- src/bin/sh/expand.c:1.132	Wed Apr 10 08:13:11 2019
+++ src/bin/sh/expand.c	Tue Oct  8 03:52:44 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: expand.c,v 1.132 2019/04/10 08:13:11 kre Exp $	*/
+/*	$NetBSD: expand.c,v 1.133 2019/10/08 03:52:44 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)expand.c	8.5 (Berkeley) 5/15/95";
 #else
-__RCSID("$NetBSD: expand.c,v 1.132 2019/04/10 08:13:11 kre Exp $");
+__RCSID("$NetBSD: expand.c,v 1.133 2019/10/08 03:52:44 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -1792,24 +1792,44 @@ match_charclass(const char *p, wchar_t c
 	char name[20];
 	char *nameend;
 	wctype_t cclass;
+	char *q;
 
 	*end = NULL;
 	p++;
+	q = [0];
 	nameend = strstr(p, ":]");
 	if (nameend == NULL || nameend == p)	/* not a valid class */
 		return 0;
 
-	if (!is_alpha(*p) || strspn(p,		/* '_' is a local extension */
-	"0123456789"  "_"
-	"abcdefghijklmnopqrstuvwxyz"
-	"ABCDEFGHIJKLMNOPQRSTUVWXYZ") != (size_t)(nameend - p))
+	if (*p == CTLESC) {
+		if (*++p == CTLESC)
+			return 0;
+		if (p == nameend)
+			return 0;
+	}
+	if (!is_alpha(*p))
 		return 0;
+	while (p < nameend) {
+		if (*p == CTLESC) {
+			p++;
+			if (p == nameend)
+return 0;
+		}
+		if (!is_in_name(*p))	/* '_' is a local extension */
+			return 0;
+		if (q < [sizeof name])
+			*q++ = *p++;
+		else
+			p++;
+	}
 
 	*end = nameend + 2;		/* committed to it being a char class */
-	if ((size_t)(nameend - p) >= sizeof(name))	/* but too long */
-		return 0;/* so no match */
-	memcpy(name, p, nameend - p);
-	name[nameend - p] = '\0';
+
+	if (q < [sizeof name])	/* a usable name found */
+		*q++ = '\0';
+	else/* too long, valid, but no match */
+		return 0;
+
 	cclass = wctype(name);
 	/* An unknown class matches nothing but is valid nevertheless. */
 	if (cclass == 0)



CVS commit: src/bin/sh

2019-10-07 Thread Robert Elz
Module Name:src
Committed By:   kre
Date:   Tue Oct  8 03:52:44 UTC 2019

Modified Files:
src/bin/sh: expand.c

Log Message:
Open code the validity test & copy of the character class name in
a bracket expression in a pattern (ie: [[:THISNAME:]]).   Previously
the code used strspn() to look for invalid chars in the name, and
then memcpy(), now we do the test and copy a character at a time.
This might, or might not, be faster, but it now correctly handles
\ quoted characters in the name (' and " quoting were already
dealt with, \ was too in an earlier version, but when the \ handling
changes were made, this piece of code broke).

Not exactly a vital bug fix (who writes [[:\alpha:]] or similar?)
but it should work correctly regardless of how obscure the usage is.

Problem noted by Harald van Dijk

XXX pullup -9


To generate a diff of this commit:
cvs rdiff -u -r1.132 -r1.133 src/bin/sh/expand.c

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



Re: CVS commit: src/bin/sh

2018-12-03 Thread Robert Elz
Date:Mon, 3 Dec 2018 10:53:29 +
From:"Martin Husemann" 
Message-ID:  <20181203105329.45433f...@cvs.netbsd.org>

  | Log Message:
  | Make pendingsigs forward declaration match the definition.

Grunge, sorry about that - I assume that's what went wrong with
the alpha build (I was waiting till the links to the logs appear to
see what happened with that...)

kre



Re: CVS commit: src/bin/sh

2018-03-18 Thread Iain Hibbert
On Fri, 16 Mar 2018, Robert Elz wrote:

> Date:Fri, 16 Mar 2018 17:24:55 +0300
> From:Valery Ushakov 
> Message-ID:  <20180316142455.gf3...@pony.stderr.spb.ru>
> 
>   | mdoc.samples(7) is a very handy reference.
> 
> For me that is the same as mdoc(7) (a copy of the same
> man page) - is that what is intended (I see mdoc.samples(7) has
> 2 links - I did not chase down and find what the other is, except
> to know it is not mdoc(7) which is a copy)

This reminds me actually; I noticed a while ago that mdoc(7) is actually a 
copy (as you suggest) of groff_mdoc(7) whereas the mdoc.samples(7) Valery 
suggests is a link to groff_mdoc(7)

but as far as I know we don't use groff for our mandoc anymore.. we also 
have mandoc_mdoc(7) which I think would be the correct manpage for the 
mandoc(1) implementation we use. Is it time to switch these around?

iain


Re: CVS commit: src/bin/sh

2018-03-17 Thread Robert Elz
Date:Sun, 18 Mar 2018 03:29:54 +0300
From:Valery Ushakov 
Message-ID:  <20180318002954.gl3...@pony.stderr.spb.ru>


  | At this point I'm inclined to just chalk it up to English culture
  | being nominalistic par excelence and give up any further attempts to
  | find common viewpoint.

That's probably the right approach, for both of us - after all, what we're
discussing is really not very important to anything - it does not really
matter what the man page - I hesitate to call it section, as in man
pages "sections" have a different meaning, but ignoring that, that
is what it is, so - section is called, nor where it is placed.   As long
as the information is there, can be found, and is correct (and ideally
comprehensible as well) that's what really matters.

All the rest is just window dressing, about which there can be
a myriad of opinions, no-one is really right, or wrong, all just have
different opinions - continuing to argue ("discuss") really achieves
nothing, as no-one can really provide any particularly good reason
for their opinion, beyond that they believe it to be correct.

  | If there are no objections, I'd like to move that material as-is
  | (including LINENO section) accordingly and drop the ENVIRONMENT
  | section.

If you move text around, take care to find the "above" and "below" and
similar references scattered elsewhere and make sure they're still
appropriate as well.

And this would probably be better mentioned first on some list that
isn't source-changes-d, and with a Subject that is more informative.

kre



Re: CVS commit: src/bin/sh

2018-03-17 Thread Valery Ushakov
On Sun, Mar 18, 2018 at 04:12:16 +0700, Robert Elz wrote:

> Date:Sat, 17 Mar 2018 22:52:51 +0300
> From:Valery Ushakov 
> Message-ID:  <20180317195251.gj3...@pony.stderr.spb.ru>
> 
>   | Environment variable is a name/value pair stored in an environment
> 
> If you stopped there I'd agree with you.
> 
>   | that a C program can access via getenv() call.
> 
> That's one way - not the only way though, and isn't really material
> 
>   | Some C functions check
>   | environment variables to change their behaviour.
> 
> And this is completely irrelevant - all kinds of different things
> can be done with them, as above, they are just name/value
> pairs.
>
>   | Yes, shell "inherits" environment variables so that they become
>   | exported shell variables. 
> 
> Where shells are a bit different than most other programs, is that
> most others simply pass through the environment (of which the
> environment vars are a part - often the whole but not necessarily)
> with some of them making small changes.
> 
> The shells on the other hand completely dominate the environment,
> generally consuming what is there at startup, and creating a whole
> new environment for the programs that they run.
> 
> There is no real distinction between which shell variables are
> environment variables and which are not - any might be - all of them
> might be.
>
> If you're thinking about things purely as a C coder, writing C code,
> then yes, there's a difference, but to the user of the shell, there
> isn't - all the vars in every sh script are potentially environment
> variables, and (if the script wants) can be inherited from, and
> exported to, other programs through the environment.  Trying to
> split them up is futile.

We must have so completely different ways of thinking about this,
because we take the same premises and then conclude from them to the
complete opposites.

IIRC, at the very beginning of Farmer's "To Your Scattered Bodies Go",
when all the people find themselves in the Riverworld, one lady
notices that where everyone is naked, nobody is.  To me it's precisely
because shell dominates the environment that it doesn't make any sense
to discuss these variables in the ENVIRONMENT section or to think of
them as, primarily, environment variables.

At this point I'm inclined to just chalk it up to English culture
being nominalistic par excelence and give up any further attempts to
find common viewpoint.


> Once again, what the section heading in the man page should be
> should reflect more what makes the man page more usable, rather than
> any meaningless debate about what is, and what is not, an
> environment variable - it is even less important (a point you made
> about a different issue a day or two ago) which mdoc markup macro
> gets used to refer to them (Dv vs Ev vs Li ...)

I'd say that discussing shell variables in a sub-section right next to
the sub-section on parameter substitution (like ksh(1) or bash(1)
manuals do) makes it more usable.

If there are no objections, I'd like to move that material as-is
(including LINENO section) accordingly and drop the ENVIRONMENT
section.

-uwe


Re: CVS commit: src/bin/sh

2018-03-17 Thread Robert Elz
Date:Sat, 17 Mar 2018 22:52:51 +0300
From:Valery Ushakov 
Message-ID:  <20180317195251.gj3...@pony.stderr.spb.ru>

  | Environment variable is a name/value pair stored in an environment

If you stopped there I'd agree with you.

  | that a C program can access via getenv() call.

That's one way - not the only way though, and isn't really material

  | Some C functions check
  | environment variables to change their behaviour.

And this is completely irrelevant - all kinds of different things
can be done with them, as above, they are just name/value
pairs.

  | Yes, shell "inherits" environment variables so that they become
  | exported shell variables. 

Where shells are a bit different than most other programs, is that
most others simply pass through the environment (of which the
environment vars are a part - often the whole but not necessarily)
with some of them making small changes.

The shells on the other hand completely dominate the environment,
generally consuming what is there at startup, and creating a whole
new environment for the programs that they run.

There is no real distinction between which shell variables are environment
variables and which are not - any might be - all of them might be.

If you're thinking about things purely as a C coder, writing C code, then
yes, there's a difference, but to the user of the shell, there isn't - all the
vars in every sh script are potentially environment variables, and (if the
script wants) can be inherited from, and exported to, other programs
through the environment.   Trying to split them up is futile.

Once again, what the section heading in the man page should be
should reflect more what makes the man page more usable, rather
than any meaningless debate about what is, and what is not, an
environment variable - it is even less important (a point you made about
a different issue a day or two ago) which mdoc markup macro gets
used to refer to them (Dv vs Ev vs Li ...)

kre



Re: CVS commit: src/bin/sh

2018-03-17 Thread Robert Elz
Date:Sat, 17 Mar 2018 23:10:36 +0300
From:Valery Ushakov 
Message-ID:  <20180317201036.gk3...@pony.stderr.spb.ru>

  | [this probably belong to tech-userlevel]

Perhaps - we don't really have a good place to discuss the doc.

  | ksh(1) has "Parameters" sections
  | bash(1) has a separate section "Shell Variables".

There are many different ways the doc could be structured,
zsh has a whole set of man pages rather than just one (all
three of those shells have rather more to document than our
sh has though).

What is the best heading to use, and what order to include
everything is a complex (and debatable) issue - it would be
easier (as it would matter less) if the man pages were to
return to their original purpose of simply being technical
specifications - but would also mean having user guides
as well (as the original Bell Labs, and even the CSRG
distributions had).   But that we have let slip - we don't
really have people interested in working on the latter.

So, the man pages need to serve both roles,. and that makes
them messier.

kre



Re: CVS commit: src/bin/sh

2018-03-17 Thread Valery Ushakov
[this probably belong to tech-userlevel]

On Sat, Mar 17, 2018 at 03:46:44 +0300, Valery Ushakov wrote:

> That reminds me...  We currently abuse ENVIRONMENT section to document
> variables like PS1 or HISTSIZE which are quite obviously not
> environment variables.  I think most of them should be moved out of
> .Sh ENVIRONMENT into a separate .Ss section inside .Sh DESCRIPTION
> (and the LINENO section should moved into DESCRIPTION as well).

ksh(1) has "Parameters" sections where it talks about parameter
substitution, special parameters and other variables set/used by the
shell.

bash(1) has a separate section "Shell Variables".

-uwe


Re: CVS commit: src/bin/sh

2018-03-17 Thread Valery Ushakov
On Sat, Mar 17, 2018 at 23:10:38 +0700, Robert Elz wrote:

> Date:Sat, 17 Mar 2018 16:18:58 +0300
> From:Valery Ushakov 
> Message-ID:  <20180317131858.gi3...@pony.stderr.spb.ru>
> 
>   | Exactly.  And when all of them are, it kinda defeats the purpose of
>   | them being in the ENVIRONMENT section.
> 
> Why?What exactly do you think an environment variable is?

Environment variable is a name/value pair stored in an environment
that a C program can access via getenv() call.  Some C functions check
environment variables to change their behaviour.

Yes, shell "inherits" environment variables so that they become
exported shell variables.  But e.g. the value of PS1 affects shell
behaviour regardless of whether PS1 was inhereted from environment or
set in the current shell.  Any pertinent documentation that describes
PS1 describes *shell* variable PS1, regardless of its exported status.


>   |  Like, CFLAGS is not an "environment variable" just because 
>   | $ CFLAGS=-g make
> 
> I disagree, that is exactly what it is.

I was too brief here, but the elaboration of the argument I had in
mind is basically the same as above.  The fact that CFLAGS is passed
to this make invocation via environment is completely accidental and
doesn't any meanignful way affects the semantic of CFLAGS as a make
variable.  I thought that was obvious enough and I also wanted to use
another example of a program that has notion of "variables" and that
can inherit/export them.


-uwe


Re: CVS commit: src/bin/sh

2018-03-17 Thread Robert Elz
Date:Sat, 17 Mar 2018 16:18:58 +0300
From:Valery Ushakov 
Message-ID:  <20180317131858.gi3...@pony.stderr.spb.ru>

  | Exactly.  And when all of them are, it kinda defeats the purpose of
  | them being in the ENVIRONMENT section.

Why?What exactly do you think an environment variable is?

  |  Like, CFLAGS is not an "environment variable" just because 
  | $ CFLAGS=-g make

I disagree, that is exactly what it is.

  | I probably should have used RANDOM and SECONDS as my examples :)

Those are more like LINENO, true - they can be exported to the environ,
but doing so isn't very useful...

kre



Re: CVS commit: src/bin/sh

2018-03-17 Thread Valery Ushakov
On Sat, Mar 17, 2018 at 09:05:37 +0700, Robert Elz wrote:

> Date:Sat, 17 Mar 2018 03:46:44 +0300
> From:Valery Ushakov 
> Message-ID:  <20180317004644.gh3...@pony.stderr.spb.ru>
> 
>   | That reminds me...  We currently abuse ENVIRONMENT section to document
>   | variables like PS1 or HISTSIZE which are quite obviously not
>   | environment variables.
> 
> Actually they are - those two particularly.   In principle, essentially
> all shell variables are (potentially) environment variables - all they
> need to be is either in the environ when the shell starts, or be the
> subject of export (or set -a).

Exactly.  And when all of them are, it kinda defeats the purpose of
them being in the ENVIRONMENT section.  Like, CFLAGS is not an
"environment variable" just because 

$ CFLAGS=-g make

is something that one would commonly use.


> But it is very common for PS1 and HISTSIZE to be imported from
> the environment, and exported again

I probably should have used RANDOM and SECONDS as my examples :)


-uwe


Re: CVS commit: src/bin/sh

2018-03-16 Thread Robert Elz
Date:Sat, 17 Mar 2018 03:46:44 +0300
From:Valery Ushakov 
Message-ID:  <20180317004644.gh3...@pony.stderr.spb.ru>

  | That reminds me...  We currently abuse ENVIRONMENT section to document
  | variables like PS1 or HISTSIZE which are quite obviously not
  | environment variables.

Actually they are - those two particularly.   In principle, essentially
all shell variables are (potentially) environment variables - all they
need to be is either in the environ when the shell starts, or be the
subject of export (or set -a).

But it is very common for PS1 and HISTSIZE to be imported from
the environment, and exported again - if sh were a true posix
shell that would be the normal way for them to be set in all but
login shells.   But that's how I set that kind of var when I run
an xterm - I do "HISTSIZE=nnn PS1='$ '  xterm -ut -geom ..."
and the HISTSIZE (etc) passed in the env to xterm get passed
down to the shell, and set that way.   [Different xterms get different
settings of course, so the shells run in them get configured
differently, to meet the needs of whatever that window is to
be used for.]

LINENO (which was the subject of an earlier commit) is a trickier
case - it can be exported to the environment, but it isn't imported
from there (though if it is in the environ of the shell when it starts,
it will be exported again - just the value in the environ is ignored.)
Having it in the environ is a rare (and not so useful) case.

  I think most of them should be moved out of
  | .Sh ENVIRONMENT into a separate .Ss section inside .Sh DESCRIPTION

I will leave that to the people who understand the principles of
the NetBSD man page layout.

  | (and the LINENO section should moved into DESCRIPTION as well).

That too.   The content (text) is what matters more to me,  where in the
man page it appears, and what kind of markup macros are used is of
far less interest.   (This also does not imply that I am satisfied with
the content of the section, or that the other issues are unimportant.)

kre



Re: CVS commit: src/bin/sh

2018-03-16 Thread Valery Ushakov
On Fri, Mar 16, 2018 at 23:58:30 +0700, Robert Elz wrote:

> Date:Fri, 16 Mar 2018 17:24:55 +0300
> From:Valery Ushakov 
> Message-ID:  <20180316142455.gf3...@pony.stderr.spb.ru>
> 
>   | mdoc.samples(7) is a very handy reference.
> 
> For me that is the same as mdoc(7) (a copy of the same
> man page) - is that what is intended (I see mdoc.samples(7) has
> 2 links - I did not chase down and find what the other is, except
> to know it is not mdoc(7) which is a copy) and while I agree that it is
> useful, it isn't all that useful - for example, it says that Dv is
> for:
>   A variable (or constant) which is defined in an include file
> 
> And so which would never be appropriate in sh(1) for which
> include files are irrelevant.
>
> This is one of the reasons I have typically avoided mdoc -- while
> I understand and agree with the intent, much of what it provides
> is far too specific, and leaves too many cases fall through the
> cracks.   Defined variables (which I would extend beyond include
> files to also allow anything #define'd - in an include file or not)
> environment variables, arguments, internal commands, command
> modifiers (and flags) all have macros, just just simple variables,
> which are none of those, and constants, do not seem to have a
> macro of their own - and just fall back on the catch all Li (literal).
>
> Given that, was changing .Li 0 into .Dv 0 really the right thing to
> do?   "0" is certainly not a defined variable...(changing the
> input to whatever just happens to generate the output desired,
> regardless of semantics seems like cheating to me.)

Right, mdoc vocabulary is defined sometimes in a bit too C-centric
terms, however even its own man page has a passage or two that
indicate that common sense should be used when it's applied to other
languages.  Defining wast hair-splitting vocabularies is possible, but
do we really want a version of DocBook for troff? :)

IMO, .Dv is the perfect match for shell parameters that has defined
semantic associated with them.

That reminds me...  We currently abuse ENVIRONMENT section to document
variables like PS1 or HISTSIZE which are quite obviously not
environment variables.  I think most of them should be moved out of
.Sh ENVIRONMENT into a separate .Ss section inside .Sh DESCRIPTION
(and the LINENO section should moved into DESCRIPTION as well).

-uwe


Re: CVS commit: src/bin/sh

2018-03-16 Thread Robert Elz
Date:Fri, 16 Mar 2018 17:24:55 +0300
From:Valery Ushakov 
Message-ID:  <20180316142455.gf3...@pony.stderr.spb.ru>

  | mdoc.samples(7) is a very handy reference.

For me that is the same as mdoc(7) (a copy of the same
man page) - is that what is intended (I see mdoc.samples(7) has
2 links - I did not chase down and find what the other is, except
to know it is not mdoc(7) which is a copy) and while I agree that it is
useful, it isn't all that useful - for example, it says that Dv is
for:
A variable (or constant) which is defined in an include file

And so which would never be appropriate in sh(1) for which
include files are irrelevant.

This is one of the reasons I have typically avoided mdoc -- while
I understand and agree with the intent, much of what it provides
is far too specific, and leaves too many cases fall through the
cracks.   Defined variables (which I would extend beyond include
files to also allow anything #define'd - in an include file or not)
environment variables, arguments, internal commands, command
modifiers (and flags) all have macros, just just simple variables,
which are none of those, and constants, do not seem to have a
macro of their own - and just fall back on the catch all Li (literal).

Given that, was changing .Li 0 into .Dv 0 really the right thing to
do?   "0" is certainly not a defined variable...(changing the
input to whatever just happens to generate the output desired,
regardless of semantics seems like cheating to me.)

kre



Re: CVS commit: src/bin/sh

2018-03-16 Thread Valery Ushakov
On Fri, Mar 16, 2018 at 18:59:49 +0700, Robert Elz wrote:

> Date:Fri, 16 Mar 2018 14:41:22 +0300
> From:Valery Ushakov 
> Message-ID:  <20180316114122.gd3...@pony.stderr.spb.ru>
> 
>   |  Please, add them back, and ideally add them also to
>   | the references to the parameters that you changed
> 
> Done.  Including one or two that you had not marked I think.
> 
> I also fixed the @ that should have been $@.
> 
> I will also change the Li to Dv in the ones that should be
> changed (mdoc markup is not my thing - I use whatever
> seems to work)

Thanks!

mdoc.samples(7) is a very handy reference.

-uwe


Re: CVS commit: src/bin/sh

2018-03-16 Thread Robert Elz
Date:Fri, 16 Mar 2018 14:15:47 +0300
From:Valery Ushakov 
Message-ID:  <20180316111547.gc3...@pony.stderr.spb.ru>

  | Or:
  |
  | When a shell function is executed, all of the shell positional
  | parameters (except $0, which remains unchanged) are set to [...]
  |
  | where 0 is a special parameter, not a positional parameter, to begin
  | with, and where the principle you refer to in your mail requires
  | "except 0", without the $ sign.

Yes, I was fixing all that while you were sending this message.

  | Also some of them are in .Dq and some not, so
[...]
  | but may be it would be a good idea to
  | mark up all of them in this (or some other) way consistently.

Yes, probably, but that is, I think, for some other day.

sh(1) still needs lots of work - thanks for all you have done in the
past few days.

  | I also try to add the form with the dollar sign in a comment for the
  | occurences I stumble upon just to make them more searchable in the
  | source at least (cf. "far more zeroes" :).

Wsh I had seen that before I deleted them all...   but they should be back
now I think (along with a comment on the first such comment commenting
on what the comment is commenting on).
[And *no-one* make comments about ending sentences with
 prepositions!]

kre



Re: CVS commit: src/bin/sh

2018-03-16 Thread Robert Elz
Date:Fri, 16 Mar 2018 14:41:22 +0300
From:Valery Ushakov 
Message-ID:  <20180316114122.gd3...@pony.stderr.spb.ru>

  | The yak is much more entangled now, at least its source form is!
  |
  | -.Dv 0 \" $0
  | +.Dv 0
  |
  | Why did you delete roff *comments* with $0 and the like?! 

I had not seen your earlier message - I (wrongly) assumed that
those were ones that (had you been ready to change) were planned
to switch from 0 to $0 (etc).

  |  Please, add them back, and ideally add them also to
  | the references to the parameters that you changed

Done.  Including one or two that you had not marked I think.

I also fixed the @ that should have been $@.

I will also change the Li to Dv in the ones that should be
changed (mdoc markup is not my thing - I use whatever
seems to work)

kre



Re: CVS commit: src/bin/sh

2018-03-16 Thread Valery Ushakov
On Fri, Mar 16, 2018 at 14:41:22 +0300, Valery Ushakov wrote:

> -.Li $1 ,
> +.Li 1 ,

PS: Also, the semantically correct change is from .Li $1 to .Dv 1

-uwe


Re: CVS commit: src/bin/sh

2018-03-16 Thread Valery Ushakov
On Fri, Mar 16, 2018 at 11:19:24 +, Robert Elz wrote:

> Module Name:  src
> Committed By: kre
> Date: Fri Mar 16 11:19:24 UTC 2018
> 
> Modified Files:
>   src/bin/sh: sh.1
> 
> Log Message:
> Give the yak a quick trim and shave, and make one or two minor
> wording changes (which are, hopefully, improvements).

The yak is much more entangled now, at least its source form is!

-.Dv 0 \" $0
+.Dv 0

Why did you delete roff *comments* with $0 and the like?!  That makes
it MUCH harder to search the roff sources for single character
parameter names.  Please, add them back, and ideally add them also to
the references to the parameters that you changed

-.Li $1 ,
+.Li 1 ,

-uwe


Re: CVS commit: src/bin/sh

2018-03-16 Thread Valery Ushakov
On Fri, Mar 16, 2018 at 17:11:09 +0700, Robert Elz wrote:

> Date:Thu, 15 Mar 2018 01:20:43 +
> From:"Valeriy E. Ushakov" 
> Message-ID:  <20180315012043.c3d92f...@cvs.netbsd.org>
> 
>   | The manual is still rather inconsistent e.g. when referring to
>   | parameters where it randomly uses both $0 and 0 or $@ and @ - but I'm
>   | not shaving that yak at least for now.
> 
> Please be careful if you get cold, and decide you need the wool.
> 
> I have just done a brief scan (of the @ uses at least) and I think
> at least most of them are correct as they are (at least, I did not
> see any that are not - but I did not look carefully at every one.)
> 
> The man page uses just "@" when it is referring to the name of
> the special parameter, and "$@" when it is referring to the results
> of expanding that special paramater, so it is correct to say
> 
>   $@ expands to ...
> but would not be to say
>   @ expands to
> as @ doesn't expand to anything, it is just a character.
> 
> Similarly,
>   the special parameter @ gives a list of ...
> (or "expands to ...") is correct,. but
>   the special parameter $@ gives ...
> would not be, as $@ is not a special parameter (or
> not normally.)
> 
> $0 is also the expansion of the special parameter 0,
> it would have the same usage rules (though I did not
> look for that one - there are far more 0's in the man page
> than @'s!)

Yes, I understand that.  Though there are still some problematic (at
least to my eye) wording in that area.  E.g.

If there are no positional parameters, the expansion of @
generates zero arguments, even when @ is double-quoted.  [...]
then "$@" expands to [...]

where

when @ is double-quoted

is, pedantically, wrong.  It's not @ that is double-quoted, it's $@.


Or:

When a shell function is executed, all of the shell positional
parameters (except $0, which remains unchanged) are set to [...]

where 0 is a special parameter, not a positional parameter, to begin
with, and where the principle you refer to in your mail requires
"except 0", without the $ sign.


Also some of them are in .Dq and some not, so

A sub-shell retains the same value of $ as its parent.

but

the value of the special parameter ``!''

Now, obviously, the exclamation mark without .Dq would be very
confusing inside a sentence, but may be it would be a good idea to
mark up all of them in this (or some other) way consistently.

I also try to add the form with the dollar sign in a comment for the
occurences I stumble upon just to make them more searchable in the
source at least (cf. "far more zeroes" :).

-uwe


Re: CVS commit: src/bin/sh

2018-03-16 Thread Robert Elz
Date:Thu, 15 Mar 2018 01:20:43 +
From:"Valeriy E. Ushakov" 
Message-ID:  <20180315012043.c3d92f...@cvs.netbsd.org>

  | The manual is still rather inconsistent e.g. when referring to
  | parameters where it randomly uses both $0 and 0 or $@ and @ - but I'm
  | not shaving that yak at least for now.

Please be careful if you get cold, and decide you need the wool.

I have just done a brief scan (of the @ uses at least) and I think
at least most of them are correct as they are (at least, I did not
see any that are not - but I did not look carefully at every one.)

The man page uses just "@" when it is referring to the name of
the special parameter, and "$@" when it is referring to the results
of expanding that special paramater, so it is correct to say

$@ expands to ...
but would not be to say
@ expands to
as @ doesn't expand to anything, it is just a character.

Similarly,
the special parameter @ gives a list of ...
(or "expands to ...") is correct,. but
the special parameter $@ gives ...
would not be, as $@ is not a special parameter (or
not normally.)

$0 is also the expansion of the special parameter 0,
it would have the same usage rules (though I did not
look for that one - there are far more 0's in the man page
than @'s!)

kre



Re: CVS commit: src/bin/sh

2017-11-20 Thread maya
...now I see it's challenge mode scripting: sed only, no awk or grep.
:-)


Re: CVS commit: src/bin/sh

2017-11-20 Thread maya
On Tue, Nov 21, 2017 at 03:27:11AM +, Christos Zoulas wrote:
> The reason is that tsutsui@? added some binaries (sysctl? etc) back to
> the ramdisk. We are working trying to fix it.

I see, thanks.

Going by the dmesg posted on dmesgd.nycbug.org, it looks like for
rootdev we can use:
cat /kern/msgbuf | awk '/root on/ { print $3 }'

No idea what hw.disknames should return/where the information might be
otherwise available.


Re: CVS commit: src/bin/sh

2017-11-20 Thread Robert Elz
Date:Tue, 21 Nov 2017 03:27:11 + (UTC)
From:chris...@astron.com (Christos Zoulas)
Message-ID:  

  | We are working trying to fix it.

Regardless, upon reflection, there is no need for -X in a SMALL shell,
as used in install media, etc., and it does add some code (there are a
few new functions to support it), so it will be gone from SMALL shells
as soon as my test build completes and verifies that I didn't delete
anything important in !SMALL shells...

However, I don't routinely run, or test, SMALL shells, so if anyone notices
any odd behaviour from the sh on a boot floppy, miniroot, or similar, after
my next commit, please let me know.

The unused xioctl() function that maya@ pointed out will go in all cases.
No-one will notice that (unless you're in the habit of doing "nm /bin/sh"
and comparing with earlier versions!)

kre




Re: CVS commit: src/bin/sh

2017-11-20 Thread Christos Zoulas
In article <20171121015824.ga17...@homeworld.netbsd.org>,
  wrote:
>On Sun, Nov 19, 2017 at 03:23:02AM +, Robert Elz wrote:
>> Module Name: src
>> Committed By:kre
>> Date:Sun Nov 19 03:23:01 UTC 2017
>> 
>> Modified Files:
>>  src/bin/sh: eval.c option.list options.c output.c output.h sh.1 var.c
>> 
>> Log Message:
>> Implement the -X option - an apparent variant of -x which sends all trace
>> output to the stderr which existed when the -X option was (last) enabled.
>> It also enables tracing by enabling -x (and when reset, +X, also resets
>> the 'x' flag (+x)).  Note that it is still -x/+x which actually
>> enables/disables the trace output.   Hence "apparent variant" - what -X
>> actually does (aside from setting -x) is just to lock the trace output,
>> rather than having it follow wherever stderr is later redirected.
>
>Hi kre,
>
>This might be the reason the atari floppies are overflowing, but not
>sure.  conditionalizing the code seems to make it somewhat smaller but
>I haven't tested an atari build.
>
>I think it's enough reason to get rid of xioctl which is said to be
>unused.

The reason is that tsutsui@? added some binaries (sysctl? etc) back to
the ramdisk. We are working trying to fix it.

christos



Re: CVS commit: src/bin/sh

2017-11-20 Thread Robert Elz
Date:Tue, 21 Nov 2017 01:58:24 +
From:m...@netbsd.org
Message-ID:  <20171121015824.ga17...@homeworld.netbsd.org>

  | This might be the reason the atari floppies are overflowing,

I didn't know they were, and yes, I suppose it might be, though the
size increase from this should not be all that much (don't know how
it applies on atari though.)

  | I think it's enough reason to get rid of xioctl which is said to be
  | unused.

Yes, hadn't noticed that one, it should vanish, I will make that happen.
Hard to believe the 20 or so bytes from that will make any difference to
anything though.

kre



Re: CVS commit: src/bin/sh

2017-11-20 Thread maya
On Sun, Nov 19, 2017 at 03:23:02AM +, Robert Elz wrote:
> Module Name:  src
> Committed By: kre
> Date: Sun Nov 19 03:23:01 UTC 2017
> 
> Modified Files:
>   src/bin/sh: eval.c option.list options.c output.c output.h sh.1 var.c
> 
> Log Message:
> Implement the -X option - an apparent variant of -x which sends all trace
> output to the stderr which existed when the -X option was (last) enabled.
> It also enables tracing by enabling -x (and when reset, +X, also resets
> the 'x' flag (+x)).  Note that it is still -x/+x which actually
> enables/disables the trace output.   Hence "apparent variant" - what -X
> actually does (aside from setting -x) is just to lock the trace output,
> rather than having it follow wherever stderr is later redirected.

Hi kre,

This might be the reason the atari floppies are overflowing, but not
sure.  conditionalizing the code seems to make it somewhat smaller but
I haven't tested an atari build.

I think it's enough reason to get rid of xioctl which is said to be
unused.


Re: CVS commit: src/bin/sh

2017-06-08 Thread Robert Elz
Date:Thu, 8 Jun 2017 17:42:43 +0200
From:Joerg Sonnenberger 
Message-ID:  <20170608154243.ga25...@britannica.bec.de>

  | /bin/sh is still broken. Try to build pkgtools/cwrappers.

I have just committed a fix/workaround which allows cwrappers to build
(for me).   I kind of doubt that this is the correct fix, it is more
removing the trigger than fixing the problem the way it should be fixed.
This might decrease the accuracy of LINENO in some situations, but should
have no other side-effects.

kre



Re: CVS commit: src/bin/sh

2017-06-08 Thread Joerg Sonnenberger
On Thu, Jun 08, 2017 at 05:42:43PM +0200, Joerg Sonnenberger wrote:
> On Thu, Jun 08, 2017 at 01:12:17PM +, Robert Elz wrote:
> > Module Name:src
> > Committed By:   kre
> > Date:   Thu Jun  8 13:12:17 UTC 2017
> > 
> > Modified Files:
> > src/bin/sh: eval.c nodetypes parser.c
> > 
> > Log Message:
> > Remove some left over baggage from the LINENO v1 implementation that
> > didn't get removed with v2, and should have.   This would have had
> > (I think, without having tested it) one very minor effect on the way
> > LINENO worked in the v2 implementation, but my guess is it would have
> > taken a long time before anyone noticed...
> 
> /bin/sh is still broken. Try to build pkgtools/cwrappers.

More precisely, going back 48h results again in a working shell.

Joerg


Re: CVS commit: src/bin/sh

2017-06-08 Thread Joerg Sonnenberger
On Thu, Jun 08, 2017 at 01:12:17PM +, Robert Elz wrote:
> Module Name:  src
> Committed By: kre
> Date: Thu Jun  8 13:12:17 UTC 2017
> 
> Modified Files:
>   src/bin/sh: eval.c nodetypes parser.c
> 
> Log Message:
> Remove some left over baggage from the LINENO v1 implementation that
> didn't get removed with v2, and should have.   This would have had
> (I think, without having tested it) one very minor effect on the way
> LINENO worked in the v2 implementation, but my guess is it would have
> taken a long time before anyone noticed...

/bin/sh is still broken. Try to build pkgtools/cwrappers.

Joerg


Re: CVS commit: src/bin/sh

2017-03-18 Thread Christos Zoulas
On Mar 19,  7:23am, k...@munnari.oz.au (Robert Elz) wrote:
-- Subject: Re: CVS commit: src/bin/sh

| If that was the aim then it failed drastically for sh, the (much bigger)
| doc in USD.doc gets cleaned by "make clean" (and regenerated again by
| just "make" though probably not if you do "make sh") - compared with that
| the sh.html1 file is just noise.

I did not say it was perfect, or at the time there were program subdirs :-)

| Still, as I said, if this change really offends anyone, feel free to switch
| it back.

It does not, it is just unexpected. As with everything time will fix it :-)

christos


Re: CVS commit: src/bin/sh

2017-03-18 Thread Robert Elz
Date:Sat, 18 Mar 2017 19:29:58 -0400
From:chris...@zoulas.com (Christos Zoulas)
Message-ID:  <20170318232958.bd31717f...@rebar.astron.com>

  | Well, this is how it is for all the rest of the programs; .html are cleaned
  | by cleandir. I think the rationale was that building docs when you are
  | debugging is just a waste of time,

If that was the aim then it failed drastically for sh, the (much bigger)
doc in USD.doc gets cleaned by "make clean" (and regenerated again by
just "make" though probably not if you do "make sh") - compared with that
the sh.html1 file is just noise.

Still, as I said, if this change really offends anyone, feel free to switch
it back.

kre



Re: CVS commit: src/bin/sh

2017-03-18 Thread Christos Zoulas
On Mar 19,  3:43am, k...@munnari.oz.au (Robert Elz) wrote:
-- Subject: Re: CVS commit: src/bin/sh

| That makes sense, but to me that would mean that clean removes files
| generated by make ("make all"), and cleandir removes files generated
| by make depend (+clean)
| 
| That's consistent - leaving one "make all" file behind after "make clean"
| just looks weird.
| 
| But if it is really important that sh.html1 be left behind after a
| "make clean", then please just revert that part of the change.

Well, this is how it is for all the rest of the programs; .html are cleaned
by cleandir. I think the rationale was that building docs when you are
debugging is just a waste of time, and having 'make clean' work when you
are debugging as fast as possible is the ultimate goal. Docs were not
considered to be part of the code those days :-)

christos


Re: CVS commit: src/bin/sh

2017-03-18 Thread Robert Elz
Date:Sat, 18 Mar 2017 14:57:41 + (UTC)
From:chris...@astron.com (Christos Zoulas)
Message-ID:  

  | It should be consistent...

That makes sense, but to me that would mean that clean removes files
generated by make ("make all"), and cleandir removes files generated
by make depend (+clean)

That's consistent - leaving one "make all" file behind after "make clean"
just looks weird.

But if it is really important that sh.html1 be left behind after a
"make clean", then please just revert that part of the change.

kre



Re: CVS commit: src/bin/sh

2017-03-18 Thread Robert Elz
Date:Sat, 18 Mar 2017 14:03:47 +0100
From:Joerg Sonnenberger 
Message-ID:  <20170318130347.gb25...@britannica.bec.de>

  | Sure, my point is primarily about the HTML man page.

Yes, I guessed that.

  | Just like you don't clean the cat page by hand.

What are you suggesting it should do?   All the other output files get
removed by make clean, is there some reason why that one in particular
should not?

The distclean target still also removes all the depend files, and similar
which clean does not.

kre




Re: CVS commit: src/bin/sh

2017-03-18 Thread Joerg Sonnenberger
On Fri, Mar 17, 2017 at 09:31:23PM +0700, Robert Elz wrote:
> Date:Fri, 17 Mar 2017 14:43:31 +0100
> From:Joerg Sonnenberger 
> Message-ID:  <20170317134331.ga14...@britannica.bec.de>
> 
>   | make distclean?
> 
> That cleans the sh.html1 file (though that one should be removed by
> make clean as well) but still didn't clean up modern debug trace files
> left around (does now) - modern means since 1995 or thereabouts...

Sure, my point is primarily about the HTML man page. Just like you don't
clean the cat page by hand.

Joerg


Re: CVS commit: src/bin/sh

2017-03-17 Thread Robert Elz
Date:Fri, 17 Mar 2017 14:43:31 +0100
From:Joerg Sonnenberger 
Message-ID:  <20170317134331.ga14...@britannica.bec.de>

  | make distclean?

That cleans the sh.html1 file (though that one should be removed by
make clean as well) but still didn't clean up modern debug trace files
left around (does now) - modern means since 1995 or thereabouts...

kre



Re: CVS commit: src/bin/sh

2017-03-17 Thread Joerg Sonnenberger
On Thu, Mar 16, 2017 at 01:09:06PM +, Robert Elz wrote:
> Module Name:  src
> Committed By: kre
> Date: Thu Mar 16 13:09:06 UTC 2017
> 
> Modified Files:
>   src/bin/sh: Makefile show.c show.h
> 
> Log Message:
> Have "make clean" remove sh.html1 and adapt it to clean trace files
> the way they have been generated the past 20 years or so...

make distclean?

Joerg


Re: CVS commit: src/bin/sh

2016-05-31 Thread Robert Elz
Date:Tue, 31 May 2016 00:11:00 +0200
From:Joerg Sonnenberger 
Message-ID:  <20160530221100.ga18...@britannica.bec.de>

  | > PR bin/43639 - check that a file being read by the '.' command
  | > is a regular file, even when it is given as a full pathname.
  | 
  | This introduces a regression for graphics/cal3d:
  | 
  | loading cache /dev/null within ltconfig
  | .: /dev/null: not a regular file
  | 
  | I think that's actually a valid use case for source.

That should be permitted now.

kre



Re: CVS commit: src/bin/sh

2016-05-30 Thread Robert Elz
Date:Tue, 31 May 2016 00:11:00 +0200
From:Joerg Sonnenberger 
Message-ID:  <20160530221100.ga18...@britannica.bec.de>

  | loading cache /dev/null within ltconfig
  | .: /dev/null: not a regular file
  | 
  | I think that's actually a valid use case for source.

Probably it is, but if so, then
 PATH=/dev; . null
should work as well.

The change made the two cases symmetric (the same test), if one is
going to change, both should change the same way.

All posix requires is a readable file, I can make it that way if that
is the consensus of what should be correct.

kre



Re: CVS commit: src/bin/sh

2016-05-30 Thread Joerg Sonnenberger
On Tue, May 03, 2016 at 03:12:40AM +, Robert Elz wrote:
> Module Name:  src
> Committed By: kre
> Date: Tue May  3 03:12:40 UTC 2016
> 
> Modified Files:
>   src/bin/sh: eval.c
> 
> Log Message:
> PR bin/43639 - check that a file being read by the '.' command
> is a regular file, even when it is given as a full pathname.

This introduces a regression for graphics/cal3d:

loading cache /dev/null within ltconfig
.: /dev/null: not a regular file

I think that's actually a valid use case for source.

Joerg


Re: CVS commit: src/bin/sh

2016-02-24 Thread Robert Elz
Date:Wed, 24 Feb 2016 09:38:40 -0500
From:"Christos Zoulas" 
Message-ID:  <20160224143840.c906df...@cvs.netbsd.org>

  | Modified Files:
  | src/bin/sh: options.c
  | 
  | Log Message:
  | If we don't have shared address space vfork fail back to using fork since
  | we are depending on the shared address space feature (from kre)

Oh, sorry, I forgot to send suggestions for commit log entries when I
sent that lot of updates...

That one makes this patch sound much more important and substantive than
it really is - what the log message there says was already done in the
shell.   All the patch does is make the initial value of the F flag reflect
that state.

That is, now you can tell if your shell was compiled without vfork()
support (if for some reason you care) by seeing whether F appears in
the value of $- (when nothing has contrived to change it, so using
a command something like)
ENV=/dev/null sh -c 'echo $-'
would do it.

kre



Re: CVS commit: src/bin/sh

2016-01-12 Thread David Laight
On Tue, Jan 05, 2016 at 01:19:36AM +, Taylor R Campbell wrote:
>Date: Mon, 4 Jan 2016 20:06:55 -0500
>From: chris...@zoulas.com (Christos Zoulas)
> 
>On Jan 5,  5:33am, k...@munnari.oz.au (Robert Elz) wrote:
>-- Subject: Re: CVS commit: src/bin/sh
> 
>|   | Does the exec'ed program know what to do with fd > 2?
>|   | Is it hard-coded, or do we specify it with -fd N?
>| 
>| More likely, if this ever was to be used, it would be /dev/fd/N
>| but certainly this is not going to be common.
> 
>Right, otherwise people would complain a lot more about ksh than
>they currently do looking at the web...
> 
> I'm pretty sure I have shell scripts that rely on /dev/fd/N working as
> an exec'd command argument for N > 2.  An example of an exec'd program
> that uses N > 2 without /dev/fd/N is gpg, with the --passphrase-fd,
> --command-fd, --status-fd options.  I would be unhappy if these broke.
> 
> ...unless I've misunderstood what this thread is about from my very
> cursory skimming of it.

Same here.
I'm pretty sure I've passed /dev/fd/3 as a command line paramter
(although you often want multiple pipes - which is too hard).

I remember some shells closing everything except 0, 1 and 2 on entry
(perhaps unless/if some command line option given).
But that is different.

I'll use 'exec n>file' (etc) to get the shell to access an extra
file (or 2 or more). Although they are probably dup'ed back before
anything is run.

Personally I think it is the job of the scripts to close
any extra fd, not the shell.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/bin/sh

2016-01-04 Thread Robert Elz
Date:Mon, 4 Jan 2016 08:26:29 -0500
From:chris...@zoulas.com (Christos Zoulas)
Message-ID:  <20160104132629.a6ee017f...@rebar.astron.com>

  | Ksh does it and it works... I must have been too aggressive.

It isn't a matter of whether it can be made to work or not for
NetBSD's build system, or whether you have hodden a bug in the
rc scripts (I'll explain why "hidden" rather than "fixed" below)
but of what is right.

In the test case you included in the commit log, I expect the
echo in test2 to work, and write to file "out" the way NetBSD's
sh (and all Bourne shell variants, since the first one) have
always done.

That is, the shell code

exec 3>&1
cmd

is (and always was intended to be) the same as the (abbreviated) C code

dup2(1, 3);
if (fork() == 0)
execlp("cmd", "cmd", (char *)0);
else
wait();

The case you use "exec 6>out" is similar, but messier to write directly
in C because there's no "open_giving_fd_n()" function, so it needs an
open/dup2/close sequence, so I'll omit that one (I'd probably
get it wrong...).  The effect should be the same however.

If I wanted fd 6 closed when test2 was run in your example, I'd write
the line that runs it as ...

sh ./test2 6>&-

We know fd 6 is open at that point, the "exec >&6" is there explicitly,
and the close hasn't happened yet.

Now I know that POSIX says you're allowed to do it the way you have changed
it to, but that's only because ksh (for whatever reason) decided to change
the semantics, and what ksh does seems to have inordinate sway with the
posix shell people...

But, they say ...

it is unspecified whether those file descriptors remain open
when the shell invokes another utility

so the way that NetBSD's shell (and Bourne shells in general) have always
done it is just as valid.

That's why you haven't "fixed" anything in the rc system by the change.
What you have done is made it rely upon (your version of) NetBSD's
implementation of that unspecified operation.   Take the scripts to any
other (rational) system and the bug (if there is one) would reappear.
On NetBSD the bug will be hidden, which makes it one of more insidious type.

If there is a bug there, it should be fixed in the scripts (by explicitly
closing fd's that have been opened using exec when they are not needed).

The "if" is not because I believe the daemons you showed need, or want,
to have those fds open, but because the rc scripts are complex, and I
have not analysed them to determine whether they are relying upon the
"keep open through exec" behaviour for some reason.  And yes, that would
also be relying on unspecified behaviour, but it is at least the ancient
historic behaviour, where it is very hard to code correctly in any sane way
if close_on_exec is forced on when it is not wanted (whereas it is trivial
to close fds that have been opened but are not wanted).

kre

ps: have you tested the shell with your change to make sure that

exec >outfile

followed by

cmd1; cmd2; cmd3

doesn't run those commands with stdout closed?



Re: CVS commit: src/bin/sh

2016-01-04 Thread Christos Zoulas
On Jan 5,  2:07am, k...@munnari.oz.au (Robert Elz) wrote:
-- Subject: Re: CVS commit: src/bin/sh

| It isn't a matter of whether it can be made to work or not for
| NetBSD's build system, or whether you have hodden a bug in the
| rc scripts (I'll explain why "hidden" rather than "fixed" below)
| but of what is right.
| 
| In the test case you included in the commit log, I expect the
| echo in test2 to work, and write to file "out" the way NetBSD's
| sh (and all Bourne shell variants, since the first one) have
| always done.
| 
| That is, the shell code
| 
|   exec 3>&1
|   cmd
| 
| is (and always was intended to be) the same as the (abbreviated) C code
| 
|   dup2(1, 3);
|   if (fork() == 0)
|   execlp("cmd", "cmd", (char *)0);
|   else
|   wait();
| 
| The case you use "exec 6>out" is similar, but messier to write directly
| in C because there's no "open_giving_fd_n()" function, so it needs an
| open/dup2/close sequence, so I'll omit that one (I'd probably
| get it wrong...).  The effect should be the same however.
| 
| If I wanted fd 6 closed when test2 was run in your example, I'd write
| the line that runs it as ...
| 
|   sh ./test2 6>&-
| 
| We know fd 6 is open at that point, the "exec >&6" is there explicitly,
| and the close hasn't happened yet.
| 
| Now I know that POSIX says you're allowed to do it the way you have changed
| it to, but that's only because ksh (for whatever reason) decided to change
| the semantics, and what ksh does seems to have inordinate sway with the
| posix shell people...
| 
| But, they say ...
| 
|   it is unspecified whether those file descriptors remain open
|   when the shell invokes another utility
| 
| so the way that NetBSD's shell (and Bourne shells in general) have always
| done it is just as valid.

Yes, the issues are:

- Is leaving descriptors open really needed? I can write the above
  example in a way it works...
- Should portable code rely on open file descriptors across exec, since
  it is unspecified behavior?
- Should there be away to control this from the shell? And if there
  is what should the default be? What should the syntax be?

| That's why you haven't "fixed" anything in the rc system by the change.
| What you have done is made it rely upon (your version of) NetBSD's
| implementation of that unspecified operation.   Take the scripts to any
| other (rational) system and the bug (if there is one) would reappear.
| On NetBSD the bug will be hidden, which makes it one of more insidious type.

Yes, but fixing this is not easy I think, without relying on a non-portable
feature of the shell to do it (or forking another subshell). What we want is:

- Setup redirections so that stdout and stderr are captured
- Execute command, where if successful the file descriptors
  are closed, and if it failed, they are kept open for further
  processing.

| If there is a bug there, it should be fixed in the scripts (by explicitly
| closing fd's that have been opened using exec when they are not needed).

If possible yes, we should definitely fix them.

| The "if" is not because I believe the daemons you showed need, or want,
| to have those fds open, but because the rc scripts are complex, and I
| have not analysed them to determine whether they are relying upon the
| "keep open through exec" behaviour for some reason.  And yes, that would
| also be relying on unspecified behaviour, but it is at least the ancient
| historic behaviour, where it is very hard to code correctly in any sane way
| if close_on_exec is forced on when it is not wanted (whereas it is trivial
| to close fds that have been opened but are not wanted).

Yes, I think that the scripts could be fixed by doubling the amount of forking..
I could be wrong though.

| kre
| 
| ps: have you tested the shell with your change to make sure that
| 
|   exec >outfile
| 
| followed by
| 
|   cmd1; cmd2; cmd3
| 
| doesn't run those commands with stdout closed?

Yup, it works. Nothing would work if it did not. The closeon-exec feature
is only for specific redirections (redirections to specific file numbers).
I.e. should I expect the following to work?

$ cat test1
#!/bin/sh
exec 6> out
echo "test" >&6
sh ./test2
exec 6>&-

$ cat test2
cat cloexec/test2
echo "test2" >&6

$ ./test1

I am open to suggestions, but I think that closing on exec is a nice
feature to have from the security POV, and it should be the default,
unless otherwise specified. I view the ksh behavior a correction
to the loose standard that was just there before close on exec did not
even exist when the shell file-descriptor redirection was invented IIRC.

christos


Re: CVS commit: src/bin/sh

2016-01-04 Thread Christos Zoulas
On Jan 5,  4:40am, k...@munnari.oz.au (Robert Elz) wrote:
-- Subject: Re: CVS commit: src/bin/sh

|   | - Is leaving descriptors open really needed? I can write the above
|   |   example in a way it works...
| 
| That one yes, but there are cases where more than one open doesn't work.

Sure, I think that there are examples where the parent/child shells use
fds > 2 to communicate, but with an exec'ed program? Does the exec'ed program
know what to do with fd > 2? Is it hard-coded, or do we specify it with -fd N?
It does not look like this is the common case.

| 
|   | - Should portable code rely on open file descriptors across exec, 
since
|   |   it is unspecified behavior?
| 
| No, portable code shouldn't, but sometimes we (or at least I) just need
| code that works for me, with no intent that it ever be used elsewhere
| (and often where no-one else would even conceivably be interested).

The knob would help for that.

| If the build system, and the rc scripts, all work with your modified sh, then
| clearly they aren't depending upon that behaviour, and in that case,
| fixing them to explicitly close the "save for later" fds should not be
| difficult (painstaking to actually do perhaps, but not difficult.)

They seem to work fine.

|   | - Should there be away to control this from the shell? And if there
|   |   is what should the default be? What should the syntax be?
| 
| Control, yes, probably.  The default should be the way NetBSD has
| always been.  It isn't a bug, and of itself it isn't a security issue
| (though, as always, sloppy code can make security holes), so we should
| retain compat with what we have always had, at least as the default case.
| 
| Syntax ... no answers right now, a different > operator perhaps (7>&!3 ?)
| (would be easiest to convert existing code if it prefers the other way),
| or a new built in command to set/reset close on exec on a (set of) fd
| (which could also manipulate other fd flags for the shell as well,
| not that most of the others would be useful - we could even call it "fcntl")

Well, ! means overwrite in re-directions... I am leaning towards the fcntl
builtin solution...

|   |   - Setup redirections so that stdout and stderr are captured
|   |   - Execute command, where if successful the file descriptors
|   | are closed, and if it failed, they are kept open for further
|   | processing.
| 
| command fd>&- ...
| 
| The cmd is run (if it can be) with fd closed.   In the shell, fd remains
| open (to be closed later, either explicitly, or upon exit) and if needed
| you can do whatever afterwards, including closing fd (or moving it back to
| stdout) if the workaround is no longer needed.
| 
| Obviously the "command" the redirect is added to needs to be the one
| (or parent of the set) where fd is not needed, not just another of the
| set of rc scripts/functions.

Perhaps, if we could do that and re-open with append. I am not sure
what happens with concurrency though.

| 
|   | Yup, it works. Nothing would work if it did not.
| 
| Really?   How frequently do we do "exec >file" in anything that
| is actually used (if the explicit number makes a difference, write
| it as "exec 1>file" - those two should be identical).

These two are identical since 0, 1, 2 fd's are never touched. OTOH
exec 6> foo isn't.

|   | The closeon-exec feature is only for specific redirections 
|   | (redirections to specific file numbers).
| 
| Yes, understood, but that's what I asked about, when the specific
| file number is 1 (or 0 or 2) - I saw nothing in the code that would
| prevent close-on-exec being set for them.

The check is in both copyfd() and in the specific fcntl case.

| I just did your test (as below) again using ksh (NetBSD's /bin/ksh, not
| the real one) and sure enough, it fails, when using fd 6.   But if I change
| it to use "1" instead of "6", then it works, so clearly ksh is doing
| different things depending upon what the fd value happens to be.   I have
| no idea how they explain that.
| 
|   | I.e. should I expect the following to work?
|   | 
|   | $ cat test1
|   | #!/bin/sh
|   | exec 6> out
|   | echo "test" >&6
|   | sh ./test2
|   | exec 6>&-
|   | 
|   | $ cat test2
|   | cat cloexec/test2
|   | echo "test2" >&6
|   | 
|   | $ ./test1
| 
| Aside from the "cat cloexec/test2" line, which I suspect might be a pasto
| in construction of your mail, otherwise I have no idea what that's about,
| then yes, on NetBSD it should work, and should write
|   test
|   test2
| to the file "out".   That's what it does on my system (which does not have
| your changed shell) right now.   Unless there is a very good reason, that
| is what it should continue to do.
| 
| Whether anyone should write code like that, which depends upon it working
| that way is a different 

Re: CVS commit: src/bin/sh

2016-01-04 Thread Robert Elz
Date:Mon, 4 Jan 2016 14:41:30 -0500
From:chris...@zoulas.com (Christos Zoulas)
Message-ID:  <20160104194130.6064f17f...@rebar.astron.com>


  | Yes, the issues are:
  | 
  | - Is leaving descriptors open really needed? I can write the above
  |   example in a way it works...

That one yes, but there are cases where more than one open doesn't work.

  | - Should portable code rely on open file descriptors across exec, since
  |   it is unspecified behavior?

No, portable code shouldn't, but sometimes we (or at least I) just need
code that works for me, with no intent that it ever be used elsewhere
(and often where no-one else would even conceivably be interested).

If the build system, and the rc scripts, all work with your modified sh, then
clearly they aren't depending upon that behaviour, and in that case,
fixing them to explicitly close the "save for later" fds should not be
difficult (painstaking to actually do perhaps, but not difficult.)

  | - Should there be away to control this from the shell? And if there
  |   is what should the default be? What should the syntax be?

Control, yes, probably.  The default should be the way NetBSD has
always been.  It isn't a bug, and of itself it isn't a security issue
(though, as always, sloppy code can make security holes), so we should
retain compat with what we have always had, at least as the default case.

Syntax ... no answers right now, a different > operator perhaps (7>&!3 ?)
(would be easiest to convert existing code if it prefers the other way),
or a new built in command to set/reset close on exec on a (set of) fd
(which could also manipulate other fd flags for the shell as well,
not that most of the others would be useful - we could even call it "fcntl")

  | - Setup redirections so that stdout and stderr are captured
  | - Execute command, where if successful the file descriptors
  |   are closed, and if it failed, they are kept open for further
  |   processing.

command fd>&- ...

The cmd is run (if it can be) with fd closed.   In the shell, fd remains
open (to be closed later, either explicitly, or upon exit) and if needed
you can do whatever afterwards, including closing fd (or moving it back to
stdout) if the workaround is no longer needed.

Obviously the "command" the redirect is added to needs to be the one
(or parent of the set) where fd is not needed, not just another of the
set of rc scripts/functions.


  | Yup, it works. Nothing would work if it did not.

Really?   How frequently do we do "exec >file" in anything that
is actually used (if the explicit number makes a difference, write
it as "exec 1>file" - those two should be identical).

  | The closeon-exec feature is only for specific redirections 
  | (redirections to specific file numbers).

Yes, understood, but that's what I asked about, when the specific
file number is 1 (or 0 or 2) - I saw nothing in the code that would
prevent close-on-exec being set for them.

I just did your test (as below) again using ksh (NetBSD's /bin/ksh, not
the real one) and sure enough, it fails, when using fd 6.   But if I change
it to use "1" instead of "6", then it works, so clearly ksh is doing
different things depending upon what the fd value happens to be.   I have
no idea how they explain that.

  | I.e. should I expect the following to work?
  | 
  | $ cat test1
  | #!/bin/sh
  | exec 6> out
  | echo "test" >&6
  | sh ./test2
  | exec 6>&-
  | 
  | $ cat test2
  | cat cloexec/test2
  | echo "test2" >&6
  | 
  | $ ./test1

Aside from the "cat cloexec/test2" line, which I suspect might be a pasto
in construction of your mail, otherwise I have no idea what that's about,
then yes, on NetBSD it should work, and should write
test
test2
to the file "out".   That's what it does on my system (which does not have
your changed shell) right now.   Unless there is a very good reason, that
is what it should continue to do.

Whether anyone should write code like that, which depends upon it working
that way is a different question, to which the answer is probably, "no",
but sometimes doing it differently is hard - harder than justified by the
requirements.

The place I use exec redirections most is when I really want to write

while read a b c
do
commands
done < file

which is fine, except when I need the result of "commands" to affect
the state of the shell outside the loop (like variable assignments).
There are some gross hacks that can be used to deal with that, but the
most elegant is to convert it to ...

exec 9<&0
exec 0

Re: CVS commit: src/bin/sh

2016-01-04 Thread Robert Elz
Date:Mon, 4 Jan 2016 16:50:18 -0500
From:chris...@zoulas.com (Christos Zoulas)
Message-ID:  <20160104215018.4017617f...@rebar.astron.com>


  | Sure, I think that there are examples where the parent/child shells use
  | fds > 2 to communicate, but with an exec'ed program?

Sure, if you're thinking some random NetBSD binary from /bin or /usr/pkg/bin
then probably not.

The exec'd programs in my case however tend to be #!/bin/sh scripts
that are tightly coupled to the script calling them.

  | Does the exec'ed program know what to do with fd > 2?
  | Is it hard-coded, or do we specify it with -fd N?

More likely, if this ever was to be used, it would be /dev/fd/N
but certainly this is not going to be common.

  | Well, ! means overwrite in re-directions...

Obviously it would not need to be ! ... N>& or N>&:M or something
that doesn't have a meaning (a sh syntax char would be better than
a "word" char like : though).

  | I am leaning towards the fcntl builtin solution...

So was I when I finished writing it.  Still am.

  | Perhaps, if we could do that and re-open with append.

Re-open what?   I'm just suggesting a specific (explicit) close in the
exact same place that a close on exec would happen.   What the shell
does internally if the cmd exec fails we cannot do anything about (or
rely upon), all that matters is the state left behind after, and in that
the fd is still open - the close only affects "command".

  | These two are identical since 0, 1, 2 fd's are never touched.

Ah, even though I looked for it, I missed the "to > 2" part.   Now I
just want to see how this gets documented in a way that makes it
seem other than foolish.

Incidentally, the other case, where you call " copyfd(f, fd, 1, fd > 2); "
the "fd > 2" bit is really redundant, the code inside copyfd (as it exists)
ends up causing a test that amounts to

if (fd > 2 && fd > 2)

which is kind of redundant.   That "fd > 2" might just as well be "1"
like in the other calls.

I guess I am going to have to set up a current system and try some of
my scripts that do this kind of thing on it, and see what happens.
I suspect this kind of sh usage is exotic enough that comparatively few
people ever use it.

Anyway, enough for now, others probably should be giving opinions
on how they think all this really should work.

kre



Re: CVS commit: src/bin/sh

2016-01-04 Thread Christos Zoulas
On Jan 5,  5:33am, k...@munnari.oz.au (Robert Elz) wrote:
-- Subject: Re: CVS commit: src/bin/sh

| The exec'd programs in my case however tend to be #!/bin/sh scripts
| that are tightly coupled to the script calling them.

Ok, this I think will break.

|   | Does the exec'ed program know what to do with fd > 2?
|   | Is it hard-coded, or do we specify it with -fd N?
| 
| More likely, if this ever was to be used, it would be /dev/fd/N
| but certainly this is not going to be common.

Right, otherwise people would complain a lot more about ksh than
they currently do looking at the web...

| Obviously it would not need to be ! ... N>& or N>&:M or something
| that doesn't have a meaning (a sh syntax char would be better than
| a "word" char like : though).

Something like it... Still I think I will start a discussion in the
very infrequently used bourne shell mailing list...

|   | I am leaning towards the fcntl builtin solution...
| 
| So was I when I finished writing it.  Still am.

Heh.

|   | Perhaps, if we could do that and re-open with append.
| 
| Re-open what?   I'm just suggesting a specific (explicit) close in the
| exact same place that a close on exec would happen.   What the shell
| does internally if the cmd exec fails we cannot do anything about (or
| rely upon), all that matters is the state left behind after, and in that
| the fd is still open - the close only affects "command".

exec 7>&- 8>&-
command
# reopen 7 and 8 and point them to what they were before.

Now the shell output when it fails to execute command, will not go
to 7 and 8 as it was expected.

| Ah, even though I looked for it, I missed the "to > 2" part.   Now I
| just want to see how this gets documented in a way that makes it
| seem other than foolish.

I would say something:

Shell file-descriptor redirections are marked close-on-exec unless the
descriptors they point to refer to the standard input, output, or error.

| Incidentally, the other case, where you call " copyfd(f, fd, 1, fd > 2); "
| the "fd > 2" bit is really redundant, the code inside copyfd (as it exists)
| ends up causing a test that amounts to
| 
|   if (fd > 2 && fd > 2)
| 
| which is kind of redundant.   That "fd > 2" might just as well be "1"
| like in the other calls.

Totally redundant.

| I guess I am going to have to set up a current system and try some of
| my scripts that do this kind of thing on it, and see what happens.
| I suspect this kind of sh usage is exotic enough that comparatively few
| people ever use it.

Please do.

| Anyway, enough for now, others probably should be giving opinions
| on how they think all this really should work.

I was waiting for that but I am getting radio silence.

christos


Re: CVS commit: src/bin/sh

2016-01-04 Thread Taylor R Campbell
   Date: Mon, 4 Jan 2016 20:06:55 -0500
   From: chris...@zoulas.com (Christos Zoulas)

   On Jan 5,  5:33am, k...@munnari.oz.au (Robert Elz) wrote:
   -- Subject: Re: CVS commit: src/bin/sh

   |   | Does the exec'ed program know what to do with fd > 2?
   |   | Is it hard-coded, or do we specify it with -fd N?
   | 
   | More likely, if this ever was to be used, it would be /dev/fd/N
   | but certainly this is not going to be common.

   Right, otherwise people would complain a lot more about ksh than
   they currently do looking at the web...

I'm pretty sure I have shell scripts that rely on /dev/fd/N working as
an exec'd command argument for N > 2.  An example of an exec'd program
that uses N > 2 without /dev/fd/N is gpg, with the --passphrase-fd,
--command-fd, --status-fd options.  I would be unhappy if these broke.

...unless I've misunderstood what this thread is about from my very
cursory skimming of it.


Re: CVS commit: src/bin/sh

2014-10-19 Thread Alan Barrett

On Sun, 19 Oct 2014, Christos Zoulas wrote:
There are some TOCTOU races in this code, where something about 
the file could change in between the stat() and the open().


Well, we could try to open without O_CREAT first, for device 
nodes it should succeed, if it fails do the O_EXCL thingy. I 
think open has enough flags.


Yes, that would probably work, and it's easier than my ideas.

--apb (Alan Barrett)


Re: CVS commit: src/bin/sh

2014-10-18 Thread Alan Barrett

On Wed, 15 Oct 2014, Christos Zoulas wrote:

Modified Files:
src/bin/sh: redir.c

Log Message:
PR/48201: Miwa Susumu: Fix set -C (no clobber) for POSIX; from FreeBSD
Can't use O_EXCL because of device nodes; also truncate.


There are some TOCTOU races in this code, where something about 
the file could change in between the stat() and the open().


Some ideas:

1. Keep the new code, with its races, but also verify that st_dev 
and st_ino values remain unchanged between the stat() before 
opening the file, and fstat() after opening the file.


2. Try open() with O_EXCL first, and fall back to racy code with 
stat() only if the first open(O_EXCL) fails.  Also use fstat() to 
check that st_dev/st_ino do not change.


3. Invent one or more open(2) flags, such as O_SPECIAL for must 
be a device or other special file, must not be a plain file or a 
directory.  First try open(O_EXCL), and if that fails then try 
open(O_SPECIAL).


--apb (Alan Barrett)


Re: CVS commit: src/bin/sh

2014-10-18 Thread Christos Zoulas
In article 20141018221947.ga2...@apb-laptoy.apb.alt.za,
Alan Barrett  a...@cequrux.com wrote:
On Wed, 15 Oct 2014, Christos Zoulas wrote:
Modified Files:
  src/bin/sh: redir.c

Log Message:
PR/48201: Miwa Susumu: Fix set -C (no clobber) for POSIX; from FreeBSD
Can't use O_EXCL because of device nodes; also truncate.

There are some TOCTOU races in this code, where something about 
the file could change in between the stat() and the open().

Some ideas:

1. Keep the new code, with its races, but also verify that st_dev 
and st_ino values remain unchanged between the stat() before 
opening the file, and fstat() after opening the file.

2. Try open() with O_EXCL first, and fall back to racy code with 
stat() only if the first open(O_EXCL) fails.  Also use fstat() to 
check that st_dev/st_ino do not change.

3. Invent one or more open(2) flags, such as O_SPECIAL for must 
be a device or other special file, must not be a plain file or a 
directory.  First try open(O_EXCL), and if that fails then try 
open(O_SPECIAL).

Well, we could try to open without O_CREAT first, for device nodes
it should succeed, if it fails do the O_EXCL thingy. I think open has
enough flags.

christos



Re: CVS commit: src/bin/sh

2011-08-23 Thread David Laight
On Tue, Aug 23, 2011 at 06:04:39AM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Tue Aug 23 10:04:39 UTC 2011
 
 Modified Files:
   src/bin/sh: expand.c
 
 Log Message:
 PR/45269: Andreas Gustafsson: Instead of falling off the edge when eating 
 trailing newlines
 if the block has moved, arrange so that trailing newlines are never placed in 
 the string
 in the first place, by accumulating them and adding them only after we've 
 encountered a
 non-newline character. This allows also for more efficient appending since we 
 know how much
 we need beforehand. From FreeBSD.
 

I'm not sure the old netbsd code is wrong - after the fix in rev 1.68
(netbsd 3 timescales).
All the values used when stripping the newlines should be valid.
Maybe there is something iffy with the sequence points in some of
the earlier code - or gcc is just getting them wrong.
The string buffer being extended shouldn't make a difference - even
if it isn't the last fragment of the heap list.

I've done a test with rev 1.84 and got the string extended while adding the
output from $(...) and it was ok - but I'm using a much older gcc.

I think it might be worth looking as the asm output from gcc for
a compilation that is known to get it wrong.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/bin/sh

2011-08-23 Thread David Laight
On Tue, Aug 23, 2011 at 06:04:39AM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Tue Aug 23 10:04:39 UTC 2011
 
 Modified Files:
   src/bin/sh: expand.c
 
 Log Message:
 PR/45269: Andreas Gustafsson: Instead of falling off the edge when eating 
 trailing newlines
 if the block has moved, arrange so that trailing newlines are never placed in 
 the string
 in the first place, by accumulating them and adding them only after we've 
 encountered a
 non-newline character. This allows also for more efficient appending since we 
 know how much
 we need beforehand. From FreeBSD.

I've checked - we already had a fix for the bug that the freebsd change
was for (running back off the beginning of the buffer).

The newer gcc's are doing something else unexpected.
Possibly even gcc 4.1.

I'm not sure we should assume that this change fixes the underlying
problem.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/bin/sh

2011-01-08 Thread David Holland
On Sat, Jan 08, 2011 at 02:10:16AM +, Christos Zoulas wrote:
  Modified Files:
  src/bin/sh: histedit.c
  
  Log Message:
  Call el_source before initializing sh-specific editline properties (i.e.
  the editor type and the tab completion binding).
  
  This allows tab completion to work when a user has an ~/.editrc file.
  
  Err, this change is wrong. If you change now the editor to vi in your
  .editrc file, then it will be ignored. You also will not be able to
  bind ^I to what you want, because your change will be undone!  If
  you put bind -v in your .editrc you should know to bind tab-complete
  to ^I for vi too! What was there was on purpose. Revert and document
  the previous behavior in the man page if you want.

For the record, this is PR 44347, not 43404. 43404 was an unrelated
local problem of mine that jmmv had initially reopened instead.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/bin/sh

2011-01-07 Thread Christos Zoulas
In article 20110107222156.ede9e17...@cvs.netbsd.org,
Julio Merino source-changes-d@NetBSD.org wrote:
-=-=-=-=-=-

Module Name:   src
Committed By:  jmmv
Date:  Fri Jan  7 22:21:56 UTC 2011

Modified Files:
   src/bin/sh: histedit.c

Log Message:
Call el_source before initializing sh-specific editline properties (i.e.
the editor type and the tab completion binding).

This allows tab completion to work when a user has an ~/.editrc file.

Err, this change is wrong. If you change now the editor to vi in your
.editrc file, then it will be ignored. You also will not be able to
bind ^I to what you want, because your change will be undone!  If
you put bind -v in your .editrc you should know to bind tab-complete
to ^I for vi too! What was there was on purpose. Revert and document
the previous behavior in the man page if you want.

christos



CVS commit: src/bin/sh

2010-03-01 Thread Joerg Sonnenberger
Module Name:src
Committed By:   joerg
Date:   Mon Mar  1 21:53:59 UTC 2010

Modified Files:
src/bin/sh: sh.1

Log Message:
\\ - \e


To generate a diff of this commit:
cvs rdiff -u -r1.97 -r1.98 src/bin/sh/sh.1

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



CVS commit: src/bin/sh

2010-03-01 Thread Joerg Sonnenberger
Module Name:src
Committed By:   joerg
Date:   Mon Mar  1 21:53:59 UTC 2010

Modified Files:
src/bin/sh: sh.1

Log Message:
\\ - \e


To generate a diff of this commit:
cvs rdiff -u -r1.97 -r1.98 src/bin/sh/sh.1

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

Modified files:

Index: src/bin/sh/sh.1
diff -u src/bin/sh/sh.1:1.97 src/bin/sh/sh.1:1.98
--- src/bin/sh/sh.1:1.97	Fri Jan  1 21:46:31 2010
+++ src/bin/sh/sh.1	Mon Mar  1 21:53:58 2010
@@ -1,4 +1,4 @@
-.\	$NetBSD: sh.1,v 1.97 2010/01/01 21:46:31 wiz Exp $
+.\	$NetBSD: sh.1,v 1.98 2010/03/01 21:53:58 joerg Exp $
 .\ Copyright (c) 1991, 1993
 .\	The Regents of the University of California.  All rights reserved.
 .\
@@ -1471,7 +1471,7 @@
 	case $f in
 	a | b)	flag=$f;;
 	c)	carg=$OPTARG;;
-	\\?)	echo $USAGE; exit 1;;
+	\e?)	echo $USAGE; exit 1;;
 	esac
 done
 shift `expr $OPTIND - 1`



CVS commit: src/bin/sh

2010-02-21 Thread Christos Zoulas
Module Name:src
Committed By:   christos
Date:   Sun Feb 21 09:54:57 UTC 2010

Modified Files:
src/bin/sh: main.c

Log Message:
fix faulty logic in previous change.


To generate a diff of this commit:
cvs rdiff -u -r1.55 -r1.56 src/bin/sh/main.c

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



CVS commit: src/bin/sh

2010-02-20 Thread Christos Zoulas
Module Name:src
Committed By:   christos
Date:   Sat Feb 20 23:15:17 UTC 2010

Modified Files:
src/bin/sh: main.c

Log Message:
default to the original behavior for $ENV unless POSIXLY_CORRECT is set.


To generate a diff of this commit:
cvs rdiff -u -r1.54 -r1.55 src/bin/sh/main.c

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



CVS commit: src/bin/sh

2010-01-01 Thread David A. Holland
Module Name:src
Committed By:   dholland
Date:   Fri Jan  1 18:09:16 UTC 2010

Modified Files:
src/bin/sh: sh.1

Log Message:
fix typo


To generate a diff of this commit:
cvs rdiff -u -r1.93 -r1.94 src/bin/sh/sh.1

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



CVS commit: src/bin/sh

2010-01-01 Thread David A. Holland
Module Name:src
Committed By:   dholland
Date:   Fri Jan  1 19:34:59 UTC 2010

Modified Files:
src/bin/sh: cd.c sh.1

Log Message:
Make the cd builtin accept and ignore -P, which is a kshism that has been
allowed to leak into POSIX and selects the behavior cd already implements.
Closes PR bin/42557 and also relevant to PR pkg/42168.

I suppose this should probably be pulled up to both -4 and -5...


To generate a diff of this commit:
cvs rdiff -u -r1.39 -r1.40 src/bin/sh/cd.c
cvs rdiff -u -r1.94 -r1.95 src/bin/sh/sh.1

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



CVS commit: src/bin/sh

2010-01-01 Thread David A. Holland
Module Name:src
Committed By:   dholland
Date:   Fri Jan  1 19:51:19 UTC 2010

Modified Files:
src/bin/sh: sh.1

Log Message:
fix another typo


To generate a diff of this commit:
cvs rdiff -u -r1.95 -r1.96 src/bin/sh/sh.1

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



CVS commit: src/bin/sh

2010-01-01 Thread Thomas Klausner
Module Name:src
Committed By:   wiz
Date:   Fri Jan  1 21:46:31 UTC 2010

Modified Files:
src/bin/sh: sh.1

Log Message:
Bump date for cd -P support.


To generate a diff of this commit:
cvs rdiff -u -r1.96 -r1.97 src/bin/sh/sh.1

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



Re: CVS commit: src/bin/sh

2009-12-10 Thread Masao Uebayashi
More thought.

All these confusions come from the inability of commands to generate output
files separatedly.  This is perfect legal dependency tree because each
input/output relationship is 1:1

 +- arith.c --+
arith.y -+ +- arith.o
 +- arith.h --+

If yacc can generate only either .c / .h, like

yacc --src arith.y
yacc --hdr arith.y

We can write the rule straight.

What we need is to have wrapper commands which extracts one of outputs.  If
a command generates 3 outputs from 4 inputs, we need 3 wrappers.

Masao

-- 
Masao Uebayashi / Tombi Inc. / Tel: +81-90-9141-4635


Re: CVS commit: src/bin/sh

2009-12-09 Thread Masao Uebayashi
 This added:
 
 arith.h: arith.c
 arith.c: arith.y
 
 I'm fairly sure this doesn't have the desired effect!
 In particular if arith.h doesn't exist, but arith.c does then you'll
 get a 'no rules to create arith.h' (or similar error).
 
 At least one of those dependencies must already exist! Otherwise
 the file would never have been generated!

True.

 Thinks 
 It might be best have:
 
 arith.h: arith.y
   yacc commands
 
 arith.o: arith.h
   [ -f ${@:.o=c} ] || { rm ${@:.o=.h}; exit 1; }
   commands to compile arith.c
 
 ie having no real dependency against arith.c

Another idea would be always generate two outputs independently.

arith.c: tmp_arith_c.c:
tmp_arith_c.c: tmp_arith_c.y
tmp_arith_c.y: arith.y
cp arith.y tmp_arith_c.y

arith.h: tmp_arith_h.h:
tmp_arith_h.h: tmp_arith_h.y
tmp_arith_h.y: arith.y
cp arith.y tmp_arith_h.y

I can think of no problem of this.  At the cost of redundancy (run command
twice + some tmp files) you get accurate dependency tree.

Masao

-- 
Masao Uebayashi / Tombi Inc. / Tel: +81-90-9141-4635


Re: CVS commit: src/bin/sh

2009-10-07 Thread Alan Barrett
On Wed, 07 Oct 2009, Thomas Klausner wrote:
 For me, on current/amd64, the boot (without -s) stops and asks for a
 shell.
 I choose /bin/sh, then leave it, and I get the prompt again.
 When I replace /bin/sh with one from 20090922, boot succeeds to
 multi-user mode without these symptoms.

Does /etc/rc run at all?  You could add set -x in strategic places
to try to figure out what's going wrong.

--apb (Alan Barrett)


Re: CVS commit: src/bin/sh

2009-10-07 Thread Matthias Scheler
On Wed, Oct 07, 2009 at 10:45:53AM +0200, Alan Barrett wrote:
 On Wed, 07 Oct 2009, Thomas Klausner wrote:
  For me, on current/amd64, the boot (without -s) stops and asks for a
  shell.

I've got the same problem under NetBSD/i386.

  I choose /bin/sh, then leave it, and I get the prompt again.
  When I replace /bin/sh with one from 20090922, boot succeeds to
  multi-user mode without these symptoms.
 
 Does /etc/rc run at all?

Yes, it is run.

 You could add set -x in strategic places to try to figure out what's
 going wrong.

It breaks here:

scripts=$(for rcd in ${rc_directories:-/etc/rc.d}; do
test -d ${rcd}  echo ${rcd}/*;
done)

With the new /bin/sh the scripts variable is empty afterwards.

Kind regards

-- 
Matthias Scheler  http://zhadum.org.uk/


Re: CVS commit: src/bin/sh

2009-10-06 Thread Thomas Klausner
One of these two changes broke booting into multi-user mode.
For me, on current/amd64, the boot (without -s) stops and asks for a
shell.
I choose /bin/sh, then leave it, and I get the prompt again.
When I replace /bin/sh with one from 20090922, boot succeeds to
multi-user mode without these symptoms.
 Thomas

On Tue, Oct 06, 2009 at 07:56:58PM +, Alan Barrett wrote:
 Module Name:  src
 Committed By: apb
 Date: Tue Oct  6 19:56:58 UTC 2009
 
 Modified Files:
   src/bin/sh: mkbuiltins
 
 Log Message:
 Make this slightly more portable; it has to run on arbitary host
 platforms at build time.  Previousy, some shells were confused by
 some of the [ ... ] tests.
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.21 -r1.22 src/bin/sh/mkbuiltins
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.
 

On Tue, Oct 06, 2009 at 04:05:10PM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Tue Oct  6 20:05:10 UTC 2009
 
 Modified Files:
   src/bin/sh: eval.c
 
 Log Message:
 fix regression exit1: Don't exec the last command in a subshell if it has
 trap[0] (trap EXIT) set. Fork instead to give the shell a chance to execute
 the trap when it is done.
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.96 -r1.97 src/bin/sh/eval.c
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.