On Mon, Jul 22, 2013 at 8:56 PM, Glenn Fowler <[email protected]> wrote: > On Mon, 22 Jul 2013 20:44:40 +0200 Roland Mainz wrote: >> On Mon, Jul 22, 2013 at 8:28 PM, Roland Mainz <[email protected]> >> wrote: >> > On Mon, Jul 22, 2013 at 8:19 PM, Glenn Fowler <[email protected]> >> > wrote: >> >> On Mon, 22 Jul 2013 20:12:45 +0200 Roland Mainz wrote: >> >>> --089e0149cb8086aeb604e21d9ff5 >> >>> Content-Type: text/plain; charset=ISO-8859-1 >> >>> Attached (as "astksh20130719_cd_fix001.diff.txt") is a patch to fix >> >>> the following issues with the cd(1) buildin: >> >>> 1. error handling in cd(1) is fixed >> >>> 2. cd(1) no longer uses |stat()| to validate whether the opened file >> >>> name is a directory if |O_DIRECTORY| is set. This saves a syscall. >> >>> 3. cd(1) now opens directories first without |O_SEARCH| and if this >> >>> fails tries again with |O_SEARCH|. This is mainly to ensure the error >> >>> messages are the same as for other shells. This could should not be >> >>> moved into libast since this backwards-compatibility support is only >> >>> for the cd(1) builtin. Needed for Solaris/Illumos to boot correctly... >> >>> ;-/ >> >> >> >> I don't understand why this is specific to the cd(1) builtin >> >> it seems that any other open(O_DIRECTORY) would want to report the same >> >> error >> >> when dealing with an XATTR file >> > >> > Erm... the issue is with |O_SEARCH| and not |O_DIRECTORY|. The errno >> > code and therefore the error message from cd(1) changes... which >> > becomes an issue if you have shell code like [[ $(LC_ALL=C cd >> > nfsdir123 2>&1) == *blablamsg* ]] in the boot sequence... I don't have >> > the details (AFAIK it's |EACCESS| vs. |EPERM|) around right now and >> > need to boot Solaris to dig it out... may need a few hours until I >> > other builds in other VMs have finished so that I can shut them down >> > and boot Solaris... > >> Here is one difference (not the one I was thinking about but it >> demonstrates the problem): > >> # old ksh93 >> $ ksh93_t -c 'cd /etc/profile' >> /bin/ksh93_t[1]: cd: /etc/profile: [Not a directory] > >> # new ksh93 without fix >> $ ksh_v -c 'cd /etc/profile' >> /home/test001/bin/ksh_v: cd: /etc/profile: [Permission denied] > >> The difference is AFAIK only a problem for cd(1) ... > > thanks > for now we'll put it the ast_openat() intercept > this patch has been confusing enough to corral it in one place > for consistency across ast
Ok... David already convinced me... ... but keep the remainder of the patch intact (e.g. errno handling, dead code removal, close of |shp->pwdfd| on exit), please... ---- 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
