On Mon, Jul 8, 2013 at 12:50 PM, Irek Szczesniak <[email protected]> wrote: > 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: [snip] >> 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]>'
Thanks for testing... :-) ... I have a new version of the patch queued (testing in progress) ... which includes a fix for the reported issue that "valgrind" reports a file descriptor leak, too... David: There are several places (and we get more soon) where |fd=sh_fcntl(fd, F_dupfd_cloexec, 10)| is used to register a |fd| with the shell (preferably outside the 0-9 fd range) ... IMO it would be usefull to have a seperate function for it (to centralise the functionality, avoid confusion with |fcntl()| and other issues) ... which would be the preferred name: |sh_dupshfd()|, |sh_regfd()| ... ? :-) ---- Bye, Roland -- __ . . __ (o.\ \/ /.o) [email protected] \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer /O /==\ O\ TEL +49 641 3992797 (;O/ \/ \O;) _______________________________________________ ast-developers mailing list [email protected] http://lists.research.att.com/mailman/listinfo/ast-developers
