On 9 July 2012 10:33, Roland Mainz <roland.ma...@nrubsig.org> wrote:
> On Mon, Jul 9, 2012 at 9:48 AM, Roland Mainz <roland.ma...@nrubsig.org> wrote:
>> Hi!
>>
>> ----
>>
>> Someone from Sun^H^H^HOracle came up with the following issue when
>> prelimitary for XPG7 conformance was tested:
>> The following test script...
>> -- snip --
>> mkdir t1
>>
>> (
>> cd t1
>> touch 'real_t1'
>> ls -1
>>
>> (
>> cd ..
>> mv t1 t2
>> mkdir t1
>> )
>>
>> ls -1
>> )
>>
>> rm -Rf t1
>> rm -Rf t2
>> -- snip --
>> ... should print $'real_t1\nreal_t1\n'.
>>
>> bash4 and ksh88 work as expected:
>> -- snip --
>> $ bash /tmp/subshpwd.sh
>> real_t1
>> real_t1
>> $ ksh88 /tmp/subshpwd.sh
>> real_t1
>> real_t1
>> -- snip --
>> ... but ksh93 currently does not:
>> -- snip --
>> $ ~/bin/ksh /tmp/subshpwd.sh
>> real_t1
>> -- snip --
>>
>> The problem is that ksh88 and bash4 use seperate processes for
>> subshells (where the parent process waits for the subshell process and
>> therefore doesn't change it's cwd itself) while ksh93 currently uses
>> the logical name of the cwd to restore the previous cwd of the
>> subshell's parent.
>>
>> Attached (as "astksh_subshell_restore_cwd_xattr20120708_001.diff.txt")
>> is a patch which fixes the problem by keeping track of the cwd using a
>> fd to the cwd and uses |fchdir()| instead of |chdir()| to change and
>> restore the cwd during "cd" and subshell restore operations.
>>
>>
>> Notes:
>> - The patch is tested on Solaris and AIX (but see below about the
>> "variables.sh" test).
>>
>> - The patch relies on |O_SEARCH| which was added with XPG7. Since
>> previous XPG versions did not have that test in the shell testsuite we
>> happily assume that only platforms implementing |O_SEARCH| must be
>> able to conform to that test (all major Unix versions have |O_SEARCH|
>> in their current versions since at least two years).
>>
>> - The patch could in theory use Linux's |O_PATH| ... but as we
>> discovered by accident in
>> https://mailman.research.att.com/pipermail/ast-developers/2012q3/001512.html
>> |O_PATH| currently does not work in Linux3.0 with |fchdir()|. However
>> Linus did us a *BIG* favor and pushed ASAP a patch (see
>> http://marc.info/?l=git-commits-head&m=134170760505086&w=2) and marked
>> it for inclusion into the 3.x stable branch, e.g. most Linux
>> distributions running the 3.x kernel series will pick it up quickly
>> (within less than a couple of months). Once we have a machine with
>> |O_PATH| fixed we can revisit the issue... but we need a iffe probe
>> then (the current patch explicitly works without iffe probes).
>>
>> - The patch causes one testsuite failure...
>> -- snip --
>> test variables begins at 2012-07-09+07:55:37
>> ./src/cmd/ksh93/tests/variables.sh[456]: .:
>> /tmp/test001/tmp_3k.sgh/script: cannot open [No such file or
>> directory]
>> test variables failed at 2012-07-09+07:55:48 with exit code 1 [ 113
>> tests 1 error ]
>> -- snip --
>> ... and I have no clue where this comes from. David... do you have any
>> idea what I did wrong ?
>>
>> - The patch has support for NFSv4 attributes (XATTR). This is not
>> mandatory but would be something nice-to-have since the same patch
>> fixing the subshell-cwd-restore also allows easy support for xattr
>> support... :-)
>>
>> - A similar issue was reported by CERN (Hi Lionel ;-) last month as
>> issue where scripts running ulimit in a subshell may produce different
>> results than without ulimit. It turns out this is the same issue... if
>> ksh93 uses "ulimit" in a subshell it will |fork()| ... any usage of
>> "cd" will then no longer affect the restore operation of the parent
>
> I forgot two comments:
> 1. The patch also fixes the issue that running a builtin like "grep"
> (which may alter the cwd if run as $ grep -r ...# and is interupted
> via CTRL-C) may restore the wrong directory because it uses the
> logical name of the directory. This becomes quite nasty on NFS
> filesystems or when running in an xattr (NFSv4 extended attributes)
> directory... in that case running such a builtin will cause it to
> leave the original directory. Same applies to very bad issues when the
> original directory becomes inaccessible (e.g. |unlink()| on that dir
> as root)
> 2. Future directions: The patch can serve as basis for thread support
> in a shell where each thread can have it's own cwd (this requires that
> we stop using the global cwd and instead pass the directorie's fd
> around to all functions which |open()| files or access directories
> (e.g. |stat()|, |unlink()| etc. would be replaced with |fstatat()| and
> |unlinkat()| etc.)).
Don't have much time so only a few comments:
1. I like the patch very very much because it fixes the "no such dir"
issue if something renames the cwd directory of a subshell's parent.
BIG +1
2. I like to have this patch in an updated ksh93u because it is very
very important to us (that hint goes to David).
3. Linux O_PATH vs fchdir() brokenness is unfortunate. I've tried
terms like idiots, amateurs and WTF? but my position requires me to
label this as "unfortunate engineering mistake", topped with
bewildered regret that Linux had to reject O_SEARCH and instead invent
O_PATH which has the same semantics. Grief here that they never tested
it. More grief will come when we use Darien's CERN contracts with
Fedora and Suse to force backports for the kernel part. Yes, I love to
make some people regret this. Very.
4. Has anyone tested Linus's patch for the "unfortunate engineering mistake"?
5. Non-AIX/Solaris platforms, like Linux: Facing the "unfortunate
engineering mistake", is there a way around this, like doing a
open(".", ...) after each chdir() in b_cd() to get an handle to
current working directory? I'll make the fix available on all
platforms, right?
6. Portability: O_SEARCH/O_PATH symbols are a precondition for O_XATTR
but not the other way around? Otherwise your #ifdef logic will break.
7. Please ask David whether you can pass the cwd dir handle to
builtins using the builtin api. It'll make our own builtin code much
easier since we already mandate the ...at() api everywhere.
8. Consult with David about the O_XATTR feature. If it is too much for
ksh93u then throw it out for now.
Done for now, hate Mondays.
Lionel
_______________________________________________
ast-developers mailing list
ast-developers@research.att.com
https://mailman.research.att.com/mailman/listinfo/ast-developers
