On Mon, Jul 8, 2013 at 12:16 AM, Roland Mainz <[email protected]> wrote: > On Sat, Jul 6, 2013 at 4:31 AM, Roland Mainz <[email protected]> wrote: >> On Thu, Jul 4, 2013 at 7:47 AM, Roland Mainz <[email protected]> >> wrote: >>> On Wed, Jul 3, 2013 at 2:06 AM, Roland Mainz <[email protected]> >>> wrote: >>>> On Tue, Jul 2, 2013 at 8:04 PM, Roland Mainz <[email protected]> >>>> wrote: >>>>> On Tue, Jul 2, 2013 at 4:11 PM, Irek Szczesniak <[email protected]> >>>>> wrote: >>>>>> On Sun, Jun 30, 2013 at 11:53 PM, Roland Mainz >>>>>> <[email protected]> wrote: >>>>>>> 2013/6/28 Glenn Fowler <[email protected]>: >> [snip] >>> Attached (as "astksh20130628_solaris_fixes003.diff.txt") is an updated >>> version of the patch which fixes the issues which came up during code >>> review: >>> - Fixed error handling in cd(1) for NFSv4/CIFS/SMBFS XATTR directories >>> - Opening the history files now goes through the >>> |*at()|-emulation&&intercept code to make sure extra flags+signal >>> restart is handled properly (tested) >>> >>> BTW: No, going through the |*at()| emulation is not slower unless the >>> |fd| argument in |openat(fd,...)| differs between individual calls >>> (well... at least the code in >>> http://svn.nrubsig.org/svn/people/gisburn/code/openat_emu/openat_emu.c >>> did maintain a cache (which is valid until |fchdir()|/|chdir()| is >>> called) and AFAIK the |*at()|-emulation in libast should do the same). >> >> ... one remaining issue came up during review... some code in >> src/cmd/ksh93 (besides globbing and directory reading) still uses >> |sfopen()| ... are there ant objections that I switch them over to >> |sfopenat()| with this patch already (we have to do it anyway in the >> future to wean-off Shell_t objects from relying on the global cwd) ? > [snip] > > Attached (as "astksh20130628_solaris_fixes004.diff.txt") is the > hopefully final patch to get ast-ksh.2013-06-28 working on all > platforms. Tested so far are Solaris 11, 10, 9, 8 and SuSE 9.3 (e.g. a > Linux version which predates the |*at()| APIs). > > * Notes: > ** The new patch fixes the use of |stat(...)|/|lstat()| vs. > |fstatat(shp->pwdfd,...) to remove libshell's dependicy from the > global cwd. If this patch gets accepted then the only other places > where the global cwd is used are: > - |sfopen()| ... should be replaced with |sfopenat()| > - |pathprog()| ... should be replaced with a |*at()| version of > |pathproc()|, e.g. |pathprogat()| > - glob/file walking functions ... need to be written toi make use of > the |*at()| APIs. In theory this can give the file walkers a > performance benefit because they no longer have to work on absolute > paths or use the global cwd > - AST spawn API ... needs to use |fchdir()| after |vfork()| > > Comments/rants/feedback *very* welcome...
We've tested the patch the whole morning on a large test deployment cluster which runs a *lot* ksh93 scripts and found 0 problems. The only issue I found when I reviewed your changes and the changes Glenn made between 2013-06-13 and 2013-06-28 is this one in cd_pwd.c: - /* Move fd to a number > 10 and *register* the fd number with the shell */ - shfd = sh_fcntl(fd, F_dupfd_cloexec, 10); + /* Move fd to a number > 10 and register the fd number with the shell */ + shfd = fcntl(fd, F_dupfd_cloexec, 10); Glenn changed the code from sh_fcntl() to fcntl() which doesn't work well since shfd is no longer registered in the shell's file descriptor database. Fix that and the patch gets a 'reviewed by Irek Szczesniak <[email protected]>' Irek _______________________________________________ ast-developers mailing list [email protected] http://lists.research.att.com/mailman/listinfo/ast-developers
