On 11 July 2012 05:28, Roland Mainz <[email protected]> wrote: > On Wed, Jul 11, 2012 at 4:34 AM, Roland Mainz <[email protected]> > wrote: >> On Mon, Jul 9, 2012 at 6:16 PM, Roland Mainz <[email protected]> >> wrote: >>> On Mon, Jul 9, 2012 at 12:59 PM, Lionel Cons >>> <[email protected]> wrote: >>> [snip] >>>> Don't have much time so only a few comments: >>> [snip] >>>> 4. Has anyone tested Linus's patch for the "unfortunate engineering >>>> mistake"? >>> >>> We didn't test it (Olga and I aren't familiar with the Linux kernel) >>> ... but I'll ask around... >>> >>>> 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? >>> >>> Uhm... that will only work if we have the "read" permission for that >>> directory... |O_SEARCH| and Linux's |O_PATH| were added to address the >>> issue and allow an application to obtain a file handle to a directory >>> for which the user only has the "execute" permission. >>> But I can modify the patch to do that... in that case all platforms >>> would use the |fchdir()| codepath except for the case that a user has >>> only "execute" but no "read" permission... then we have to fall-back >>> to |chdir()|. >>> >>>> 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. >>> >>> Yes... a platform can only have |O_XATTR| if it also has |O_SEARCH|. >>> Only Linux has |O_PATH| but no |O_XATTR|. >>> >>>> 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. >>> >>> I'll ask him... >>> >>> I'll craft a new patch this evening... >> >> Attached (as "astksh_subshell_restore_cwd_xattr20120710_001.diff.txt") >> is a new version of the patch... >> ... there is one major problem: ksh93 with this patch passes all tests >> on Solaris 11 and AIX but it does not pass them on Linux... and I have >> no clue why this happens... ;-( >> >> ... David: Can you have a look, please ? > > Cancel that... I tested an old version of the patch on Linux. The > current one I passed around (and again attached to his email as > "astksh_subshell_restore_cwd_xattr20120710_001.diff.txt") works OK on > Linux, too. > Sorry for the confusion... too many systems to test... ;-/ > > David: Patch should be ready... is it OK for you or does it need more > adjustments ? > > Notes: > - The patch was designed in the assumtion that having |O_SEARCH| is > the default case... and then I tacked-on all parts which are neccesary > to get it working on all systems which do _not_ have |O_SEARCH|.
The patch works well here, even with significant positive impact on performance for scripts which use many nested subshells with cwd on NFS filesystems (the shell now keeps an open fd to the cwd of the subshell's parent so it remains cached client-side). There are two glitches in your patch: 1) You use AT_FDCWD in sh/init.c without #ifdef AT_FDCWD. 2) The testsuite needs to increase the number of fds on Solaris to 512, otherwise you will see 'functions.sh[1018]: cannot handle comsub depth > 256 in function', which is caused by ksh running out of free fd slots for the directories cwd. Lionel _______________________________________________ ast-developers mailing list [email protected] https://mailman.research.att.com/mailman/listinfo/ast-developers
