[ast-developers] cd -f $fd in a subshell doesn't restore old cwd after subshell exists...

[Roland Mainz]

Hi!

----

The following testcase...
-- snip --
redirect {basefd}<"."
touch "x5"
cd -@ "x5"
redirect {n}<"."
cd -f ${basefd}
(cd -f "$n" ; print "hello5" >"myxattr5")
/usr/bin/runat "x5" "cat myxattr5"
(cd -@ "x5" ; cat "myxattr5" )
rm "x5"
-- snip --
... shows an issue with cd -f $fd in subshells in ast-ksh.2013-12-06
on Solaris 11/B145/64bit... it seems it doesn't restore the cwd of the
parent shell level when the non-|fork()|'ing subshell terminates:
-- snip --
$ ~/bin/ksh -x xattr_cd_fd002.sh
+ command exec
+ {basefd}< .
+ touch x5
+ cd -@ x5
+ command exec
+ {n}< .
+ cd -f 12
+ cd -f 10
+ print hello5
+ 1> myxattr5
+ /usr/bin/runat x5 'cat myxattr5'
runat: cannot open x5: No such file or directory
+ cd -@ x5
xxx.sh[8]: cd: /dev/file/xattr at x5//@//: [Not a directory]
+ cat myxattr5
hello5
+ rm x5
rm: x5: not found
-- snip --

Here is a reduced testcase which should work on all platforms:
-- snip --
$ ~/bin/ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd -f $d ;
/bin/pwd) ; /bin/pwd ; true '
-- snip --

On Solaris 11/b145/AMD64/64bit and SuSE 12.3/AMD64/64bit it prints this...
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1/f1
-- snip --

... but AFAIK it should print:
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1
-- snip --

----

Bye,
Roland

[Roland Mainz]

Post by Roland Mainz
The following testcase...
-- snip --
redirect {basefd}<"."
touch "x5"
redirect {n}<"."
cd -f ${basefd}
(cd -f "$n" ; print "hello5" >"myxattr5")
/usr/bin/runat "x5" "cat myxattr5"
rm "x5"
-- snip --
... shows an issue with cd -f $fd in subshells in ast-ksh.2013-12-06
on Solaris 11/B145/64bit... it seems it doesn't restore the cwd of the
-- snip --
$ ~/bin/ksh -x xattr_cd_fd002.sh
+ command exec
+ {basefd}< .
+ touch x5
+ command exec
+ {n}< .
+ cd -f 12
+ cd -f 10
+ print hello5
+ 1> myxattr5
+ /usr/bin/runat x5 'cat myxattr5'
runat: cannot open x5: No such file or directory
+ cat myxattr5
hello5
+ rm x5
rm: x5: not found
-- snip --
-- snip --
$ ~/bin/ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd -f $d ;
/bin/pwd) ; /bin/pwd ; true '
-- snip --
On Solaris 11/b145/AMD64/64bit and SuSE 12.3/AMD64/64bit it prints this...
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1/f1
-- snip --
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1
-- snip --

Grumpf... trying again... now with Glenn's new/right email address (sorry) ...

----

Bye,
Roland

[Glenn Fowler]

this fails
ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd -f $d ; /bin/pwd) ;
/bin/pwd ; : '
this works
ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd ~{d} ; /bin/pwd) ;
/bin/pwd ; : '
and this works
ksh -cx 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd /dev/fd/$d ;
/bin/pwd) ; /bin/pwd ; : '

this quick fix to bltins/cd_pwd.c translates -f N to /dev/fd/N and goes
through the path-based code -- it should do until the shp->pwdfd code
hardens
or maybe it can simplify the logic?
--
--- /home/gsf/src/cmd/ksh93/bltins/cd_pwd.c Thu Dec 5 11:13:32 2013
+++ bltins/cd_pwd.c Tue Dec 10 22:01:54 2013
@@ -137,9 +137,25 @@
j = sfprintf(shp->strbuf2,"%s",dir);
dir = sfstruse(shp->strbuf2);
pathcanon(dir, j + 1, 0);
+#if 1
+ if (*dir!='/' && dirfd!=shp->pwdfd)
+ {
+ sfprintf(shp->strbuf,
"/dev/file/xattr@/dev/fd/%d/%s//@//",
dirfd, dir);
+ dirfd = shp->pwdfd;
+ }
+ else
+#endif
sfprintf(shp->strbuf, "/dev/file/xattr@%s//@//", dir);
dir = sfstruse(shp->strbuf);
}
+#if 1
+ else if (*dir!='/' && dirfd!=shp->pwdfd)
+ {
+ sfprintf(shp->strbuf, "/dev/fd/%d/%s", dirfd, dir);
+ dirfd = shp->pwdfd;
+ dir = sfstruse(shp->strbuf);
+ }
+#endif
#if _WINIX
if(*dir != '/' && (dir[1]!=':'))
#else

Post by Roland Mainz
Hi!
----
The following testcase...
-- snip --
redirect {basefd}<"."
touch "x5"
redirect {n}<"."
cd -f ${basefd}
(cd -f "$n" ; print "hello5" >"myxattr5")
/usr/bin/runat "x5" "cat myxattr5"
rm "x5"
-- snip --
... shows an issue with cd -f $fd in subshells in ast-ksh.2013-12-06
on Solaris 11/B145/64bit... it seems it doesn't restore the cwd of the
-- snip --
$ ~/bin/ksh -x xattr_cd_fd002.sh
+ command exec
+ {basefd}< .
+ touch x5
+ command exec
+ {n}< .
+ cd -f 12
+ cd -f 10
+ print hello5
+ 1> myxattr5
+ /usr/bin/runat x5 'cat myxattr5'
runat: cannot open x5: No such file or directory
+ cat myxattr5
hello5
+ rm x5
rm: x5: not found
-- snip --
-- snip --
$ ~/bin/ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd -f $d ;
/bin/pwd) ; /bin/pwd ; true '
-- snip --
On Solaris 11/b145/AMD64/64bit and SuSE 12.3/AMD64/64bit it prints this...
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1/f1
-- snip --
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1
-- snip --
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) roland.mainz at nrubsig.org
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL [phone]
(;O/ \/ \O;)
_______________________________________________
ast-developers mailing list
ast-developers at lists.research.att.com
http://lists.research.att.com/mailman/listinfo/ast-developers

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.research.att.com/pipermail/ast-developers/attachments/20131210/c01367d3/attachment.html>

[Irek Szczesniak]

Post by Glenn Fowler
this fails
ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd -f $d ; /bin/pwd) ;
/bin/pwd ; : '
this works
ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd ~{d} ; /bin/pwd) ;
/bin/pwd ; : '
and this works
ksh -cx 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd /dev/fd/$d ;
/bin/pwd) ; /bin/pwd ; : '
this quick fix to bltins/cd_pwd.c translates -f N to /dev/fd/N and goes
through the path-based code -- it should do until the shp->pwdfd code
hardens
or maybe it can simplify the logic?
--
--- /home/gsf/src/cmd/ksh93/bltins/cd_pwd.c Thu Dec 5 11:13:32 2013
+++ bltins/cd_pwd.c Tue Dec 10 22:01:54 2013
@@ -137,9 +137,25 @@
j = sfprintf(shp->strbuf2,"%s",dir);
dir = sfstruse(shp->strbuf2);
pathcanon(dir, j + 1, 0);
+#if 1
+ if (*dir!='/' && dirfd!=shp->pwdfd)
+ {
+ sfprintf(shp->strbuf,
+ dirfd = shp->pwdfd;
+ }
+ else
+#endif
dir = sfstruse(shp->strbuf);
}
+#if 1
+ else if (*dir!='/' && dirfd!=shp->pwdfd)
+ {
+ sfprintf(shp->strbuf, "/dev/fd/%d/%s", dirfd, dir);
+ dirfd = shp->pwdfd;
+ dir = sfstruse(shp->strbuf);
+ }
+#endif
#if _WINIX
if(*dir != '/' && (dir[1]!=':'))
#else
On Tue, Dec 10, 2013 at 5:07 PM, Roland Mainz <roland.mainz at nrubsig.org>

Post by Roland Mainz
Hi!
----
The following testcase...
-- snip --
redirect {basefd}<"."
touch "x5"
redirect {n}<"."
cd -f ${basefd}
(cd -f "$n" ; print "hello5" >"myxattr5")
/usr/bin/runat "x5" "cat myxattr5"
rm "x5"
-- snip --
... shows an issue with cd -f $fd in subshells in ast-ksh.2013-12-06
on Solaris 11/B145/64bit... it seems it doesn't restore the cwd of the
-- snip --
$ ~/bin/ksh -x xattr_cd_fd002.sh
+ command exec
+ {basefd}< .
+ touch x5
+ command exec
+ {n}< .
+ cd -f 12
+ cd -f 10
+ print hello5
+ 1> myxattr5
+ /usr/bin/runat x5 'cat myxattr5'
runat: cannot open x5: No such file or directory
+ cat myxattr5
hello5
+ rm x5
rm: x5: not found
-- snip --
-- snip --
$ ~/bin/ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd -f $d ;
/bin/pwd) ; /bin/pwd ; true '
-- snip --
On Solaris 11/b145/AMD64/64bit and SuSE 12.3/AMD64/64bit it prints this...
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1/f1
-- snip --
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1
-- snip --

I don't like the patch because the /dev/file paths are not very
portable. IMHO, if cd -f $fd is used, $PWD should return a path
relative to /proc/$$/$fd/, which is obviously not happening for
Roland's testcase.

About the original bug, does spawnvex() support directory fds? Maybe
we need a spawnvexat() function? It looks the shell is behaving
correctly, but as soon as external processes get involved they hit the
wrong directory.

Irek

PS: What happened to the patch which added shp->pwdfd support for the
builtin commands in libcmd?

[phi-vpn]

Post by Irek Szczesniak
I don't like the patch because the /dev/file paths are not very
portable. IMHO, if cd -f $fd is used, $PWD should return a path
relative to /proc/$$/$fd/, which is obviously not happening for
Roland's testcase.

I am out of context so I may mis-understand, but do you infere that
/proc is more portable than /dev ?


My OS don't have /proc

Cheers,
Phi

[Glenn Fowler]

the "AFAIK" part or roland's test has no mention of /proc or /dev/fd

Post by Roland Mainz

Post by Glenn Fowler
this fails
ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd -f $d ; /bin/pwd)

;

Post by Glenn Fowler
/bin/pwd ; : '
this works
ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd ~{d} ; /bin/pwd) ;
/bin/pwd ; : '
and this works
ksh -cx 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd /dev/fd/$d ;
/bin/pwd) ; /bin/pwd ; : '
this quick fix to bltins/cd_pwd.c translates -f N to /dev/fd/N and goes
through the path-based code -- it should do until the shp->pwdfd code
hardens
or maybe it can simplify the logic?
--
--- /home/gsf/src/cmd/ksh93/bltins/cd_pwd.c Thu Dec 5 11:13:32 2013
+++ bltins/cd_pwd.c Tue Dec 10 22:01:54 2013
@@ -137,9 +137,25 @@
j = sfprintf(shp->strbuf2,"%s",dir);
dir = sfstruse(shp->strbuf2);
pathcanon(dir, j + 1, 0);
+#if 1
+ if (*dir!='/' && dirfd!=shp->pwdfd)
+ {
+ sfprintf(shp->strbuf,
+ dirfd = shp->pwdfd;
+ }
+ else
+#endif
dir = sfstruse(shp->strbuf);
}
+#if 1
+ else if (*dir!='/' && dirfd!=shp->pwdfd)
+ {
+ sfprintf(shp->strbuf, "/dev/fd/%d/%s", dirfd, dir);
+ dirfd = shp->pwdfd;
+ dir = sfstruse(shp->strbuf);
+ }
+#endif
#if _WINIX
if(*dir != '/' && (dir[1]!=':'))
#else
On Tue, Dec 10, 2013 at 5:07 PM, Roland Mainz <roland.mainz at nrubsig.org>

Post by Roland Mainz
Hi!
----
The following testcase...
-- snip --
redirect {basefd}<"."
touch "x5"
redirect {n}<"."
cd -f ${basefd}
(cd -f "$n" ; print "hello5" >"myxattr5")
/usr/bin/runat "x5" "cat myxattr5"
rm "x5"
-- snip --
... shows an issue with cd -f $fd in subshells in ast-ksh.2013-12-06
on Solaris 11/B145/64bit... it seems it doesn't restore the cwd of the
-- snip --
$ ~/bin/ksh -x xattr_cd_fd002.sh
+ command exec
+ {basefd}< .
+ touch x5
+ command exec
+ {n}< .
+ cd -f 12
+ cd -f 10
+ print hello5
+ 1> myxattr5
+ /usr/bin/runat x5 'cat myxattr5'
runat: cannot open x5: No such file or directory
+ cat myxattr5
hello5
+ rm x5
rm: x5: not found
-- snip --
-- snip --
$ ~/bin/ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd -f $d ;
/bin/pwd) ; /bin/pwd ; true '
-- snip --
On Solaris 11/b145/AMD64/64bit and SuSE 12.3/AMD64/64bit it prints

this...

Post by Glenn Fowler

Post by Roland Mainz
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1/f1
-- snip --
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1
-- snip --

I don't like the patch because the /dev/file paths are not very
portable. IMHO, if cd -f $fd is used, $PWD should return a path
relative to /proc/$$/$fd/, which is obviously not happening for
Roland's testcase.
About the original bug, does spawnvex() support directory fds? Maybe
we need a spawnvexat() function? It looks the shell is behaving
correctly, but as soon as external processes get involved they hit the
wrong directory.
Irek
PS: What happened to the patch which added shp->pwdfd support for the
builtin commands in libcmd?

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.research.att.com/pipermail/ast-developers/attachments/20131211/4abcad8a/attachment.html>

[Irek Szczesniak]

Post by Glenn Fowler
the "AFAIK" part or roland's test has no mention of /proc or /dev/fd

Roland?

Post by Glenn Fowler
On Wed, Dec 11, 2013 at 5:40 AM, Irek Szczesniak <iszczesniak at gmail.com>

Post by Irek Szczesniak

Post by Glenn Fowler
this fails
ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd -f $d ; /bin/pwd) ;
/bin/pwd ; : '
this works
ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd ~{d} ; /bin/pwd) ;
/bin/pwd ; : '
and this works
ksh -cx 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd /dev/fd/$d ;
/bin/pwd) ; /bin/pwd ; : '
this quick fix to bltins/cd_pwd.c translates -f N to /dev/fd/N and goes
through the path-based code -- it should do until the shp->pwdfd code
hardens
or maybe it can simplify the logic?
--
--- /home/gsf/src/cmd/ksh93/bltins/cd_pwd.c Thu Dec 5 11:13:32 2013
+++ bltins/cd_pwd.c Tue Dec 10 22:01:54 2013
@@ -137,9 +137,25 @@
j = sfprintf(shp->strbuf2,"%s",dir);
dir = sfstruse(shp->strbuf2);
pathcanon(dir, j + 1, 0);
+#if 1
+ if (*dir!='/' && dirfd!=shp->pwdfd)
+ {
+ sfprintf(shp->strbuf,
+ dirfd = shp->pwdfd;
+ }
+ else
+#endif
dir = sfstruse(shp->strbuf);
}
+#if 1
+ else if (*dir!='/' && dirfd!=shp->pwdfd)
+ {
+ sfprintf(shp->strbuf, "/dev/fd/%d/%s", dirfd, dir);
+ dirfd = shp->pwdfd;
+ dir = sfstruse(shp->strbuf);
+ }
+#endif
#if _WINIX
if(*dir != '/' && (dir[1]!=':'))
#else
On Tue, Dec 10, 2013 at 5:07 PM, Roland Mainz <roland.mainz at nrubsig.org>

Post by Roland Mainz
Hi!
----
The following testcase...
-- snip --
redirect {basefd}<"."
touch "x5"
redirect {n}<"."
cd -f ${basefd}
(cd -f "$n" ; print "hello5" >"myxattr5")
/usr/bin/runat "x5" "cat myxattr5"
rm "x5"
-- snip --
... shows an issue with cd -f $fd in subshells in ast-ksh.2013-12-06
on Solaris 11/B145/64bit... it seems it doesn't restore the cwd of the
-- snip --
$ ~/bin/ksh -x xattr_cd_fd002.sh
+ command exec
+ {basefd}< .
+ touch x5
+ command exec
+ {n}< .
+ cd -f 12
+ cd -f 10
+ print hello5
+ 1> myxattr5
+ /usr/bin/runat x5 'cat myxattr5'
runat: cannot open x5: No such file or directory
+ cat myxattr5
hello5
+ rm x5
rm: x5: not found
-- snip --
-- snip --
$ ~/bin/ksh -c 'mkdir -p f1 ; redirect {d}<f1 ; /bin/pwd ; (cd -f $d ;
/bin/pwd) ; /bin/pwd ; true '
-- snip --
On Solaris 11/b145/AMD64/64bit and SuSE 12.3/AMD64/64bit it prints this...
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1/f1
-- snip --
-- snip --
/home/test001/x1
/home/test001/x1/f1
/home/test001/x1
-- snip --

I don't like the patch because the /dev/file paths are not very
portable. IMHO, if cd -f $fd is used, $PWD should return a path
relative to /proc/$$/$fd/, which is obviously not happening for
Roland's testcase.
About the original bug, does spawnvex() support directory fds? Maybe
we need a spawnvexat() function? It looks the shell is behaving
correctly, but as soon as external processes get involved they hit the
wrong directory.
Irek
PS: What happened to the patch which added shp->pwdfd support for the
builtin commands in libcmd?