tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Weirdness in /bin/sh of 8.0



    Date:        Tue, 14 Aug 2018 16:40:52 +0200
    From:        Rhialto <rhialto%falu.nl@localhost>
    Message-ID:  <20180814144052.GD5616%falu.nl@localhost>

  | On Mon 13 Aug 2018 at 07:09:41 +0700, Robert Elz wrote:
  | > This patch ...
  |
  | indeed fixes the problem for me.

Actually it is incomplete.   If you look a bit lower down after where the
patch is applied, you'll see a call to popredir() that is protected by a
test something like (I am not looking at the source at the minute)
	if (flags & REDIR_MORE)
That test needs to be removed, so the popredir() is unconditonal,
otherwise the file descriptors won't go back where they should when
they should.   That couid easily break more scripts than the code
without the patch.

I am going to completely rip out the currenr PR 48875 "fix" sometime
very soon, and replace it.   I have the replacement compiling, but
it has had only the most rudimetary testing so far, so give it a few more
days.   It is a significant change, which means anything might happen.

  | If you just have something like the following in it

Yes, I worked that out, and did that, and could see that it was getting
past the point of the bug, but that did not mean it was really working
(and the bug I left in there might have caused other issues.)

  | In the mean time I will keep the patched copy of sh as a spare but not
  | for general use.

Make the extra change!

  | You mean "need to be fixed in the script" or "need to be fixed in sh"?

Fixed in the script.   Not things which would stop it working, just stuff
done the wrong way.

Eg: somewhere in there I saw
	somevar="$@"

"$@" only has defined results when used in a conrext where field splitting
would normally occur, as despite the quotes, it produces multiple words.
variable assignments are not such a place - there a single string is needed
(and the quotes are rarely, if ever, needed).   The script should use "$*"
instead - one of few times the quotes are required (I think, I'd need to check.)

kre



Home | Main Index | Thread Index | Old Index