Re: /bin/sh redirection in 'while' loop not closed afterwards?

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

In article <20110927072628.GQ19374%snowdrop.l8s.co.uk@localhost>,
David Laight  <david%l8s.co.uk@localhost> wrote:
>On Mon, Sep 26, 2011 at 11:47:29PM +0200, Anthony Mallet wrote:
>> On Monday, at 22:23, David Laight wrote:
>> | At a guess the fd for redirection usually get opened, duped above 10, then
>> | duped to the required number very late on (in the child).
>> 
>> Yes, exactly.
>> The problem seems to appear around redir.c:144
>> 
>> fcntl() returns EBADF, 'try' is incremented and the redirection is never
>> registered in sv->renamed. Then popredir() does not undo it afterwards.
>> 
>> With a simpler redirection like "ls 9</dev/null" fcntl does not return EBADF.
>> So I'm not sure what's wrong.
>> 
>> again:
>>                      if ((i = fcntl(fd, F_DUPFD, 10)) == -1) {
>>                              switch (errno) {
>>                              case EBADF:
>>                                      if (!try) {
>>                                              openredirect(n, memory, flags);
>>                                              try++;
>>                                              goto again;
>>                                      }
>> [...]
>>                      }
>>                      if (!try) {
>>                              sv->renamed[fd] = i;
>>                              close(fd);
>>                      }
>
>I don't think that loop should be used - it looks like it is there
>for the case when all fd >= 10 are already in use.
>Since you are seeing a leaked fd above 10, that fcntl() is where
>it would come from.
>
>Traditionally sh did redirects on buildins using fork - which is why any
>shell variables they change get lost. Some shells (some ksh?) use the
>same process (posix allows both), but I thought netbsd's /bin/sh
>did fork.
>
>I'd look at where sv->renamed[] gets read.

Maybe the vfork() changes broke it a while ago. I would see if the FreeBSD
copy that does not have the vfork() changes suffers from the same issue.

christos