Re: CVS commit: src/bin/sh

[christos%zoulas.com@localhost (Christos Zoulas)]

On Jan 5,  4:40am, kre%munnari.OZ.AU@localhost (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 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<file
| 	while read a b c
| 	do
| 		commands 9<&-	# intentionally with stdin still from "file"
| 	done
| 	exec 0<&9
| 
| However, if the definition of "exec 0<" is to set close on exec on 0,
| then that is not going to work very well any more.

I think it still works, since the close happens on the forked child...

| For what it is worth, just so the example doesn't look a bit unlikely to
| be useful, for me, "file" here is often "/dev/tty" -- the script running
| this has its stdin directed from elsewhere - a pipe usually - but I need
| to interact with the script, and with commands it runs - like "more"
| (or less) perhaps.)

Let's see how it goes; if things break I will revert it. So far I don't
see anything broken.

christos