Skip to content

Commit

Permalink
Fix: Closing a FD within a comsub broke output (rhbz#1116072)
Browse files Browse the repository at this point in the history
Another Red Hat patch. "Prior to this update, the result of a
command substitution was lost if a file descriptor used for the
substitution was previously explicitly closed. With this update,
ksh no longer reuses file descriptors that were closed during the
execution of a command substitution. Now, command substitutions
work as expected in the described situation."

Prior discussion:
https://bugzilla.redhat.com/1116072

Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20140929-safefd.patch

src/cmd/ksh93/include/io.h,
src/cmd/ksh93/sh/io.c:
- Add sh_iosafefd() function to get a file descriptor that is not
  in use or otherwise occupied (including marked as closed).

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Use that function to obtain a safe FD upon restoring state when
  exiting a command substitution. I don't really know the how and
  why -- all that I/O magic is still beyond me and the code is
  uncommented as usual.

src/cmd/ksh93/tests/subshell.sh:
- Add regression test from the reproducer in the bug, reduced to
  the minimum necessary.
  • Loading branch information
McDutchie committed Sep 27, 2020
1 parent 18b3f4a commit 045fe6a
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/cmd/ksh93/include/io.h
Expand Up @@ -79,6 +79,7 @@ extern Sfio_t *sh_iostream(Shell_t*,int);
extern int sh_redirect(Shell_t*,struct ionod*,int);
extern void sh_iosave(Shell_t *, int,int,char*);
extern int sh_iovalidfd(Shell_t*, int);
extern int sh_iosafefd(Shell_t*, int);
extern int sh_inuse(Shell_t*, int);
extern void sh_iounsave(Shell_t*);
extern void sh_iounpipe(Shell_t*);
Expand Down
18 changes: 18 additions & 0 deletions src/cmd/ksh93/sh/io.c
Expand Up @@ -436,6 +436,24 @@ int sh_iovalidfd(Shell_t *shp, int fd)
return(1);
}

int sh_iosafefd(Shell_t* shp, int sfd)
{
register int fd;
while(1)
{
for(fd=0; fd < shp->topfd; fd++)
{
if (filemap[fd].save_fd==sfd || filemap[fd].orig_fd==sfd || (fcntl(sfd, F_GETFD) != -1 || errno != EBADF))
{
sfd++;
continue;
}
}
break;
}
return(sfd);
}

int sh_inuse(Shell_t *shp, int fd)
{
return(fd < shp->gd->lim.open_max && shp->fdptrs[fd]);
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/subshell.c
Expand Up @@ -679,7 +679,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
}
if(iop && sffileno(iop)==1)
{
int fd=sfsetfd(iop,3);
int fd = sfsetfd(iop,sh_iosafefd(shp,3));
if(fd<0)
{
shp->toomany = 1;
Expand Down
17 changes: 17 additions & 0 deletions src/cmd/ksh93/tests/subshell.sh
Expand Up @@ -830,5 +830,22 @@ sleep_pid=$!
((!(e = $?))) || err_exit "backtick comsub with pipe hangs (got status $e$( ((e>128)) && print -n / && kill -l "$e"))"
kill "$sleep_pid" 2>/dev/null

# ======
# https://bugzilla.redhat.com/1116072

expect='return-value'
function get_value
{
/dev/null/foo 2>/dev/null # trigger part 1: external command (even nonexistent)
exec 3>&- # trigger part 2: close file descriptor 3
print return-value # with the bug, the comsub won't output this
}
actual=$(get_value)
[[ $actual == "$expect" ]] || err_exit "\$(Comsub) failed to return output (expected '$expect', got '$actual')"
actual=${ get_value; }
[[ $actual == "$expect" ]] || err_exit "\${ Comsub; } failed to return output (expected '$expect', got '$actual')"
actual=`get_value`
[[ $actual == "$expect" ]] || err_exit "\`Comsub\` failed to return output (expected '$expect', got '$actual')"

# ======
exit $((Errors<125?Errors:125))

0 comments on commit 045fe6a

Please sign in to comment.