On Mon, Jul 9, 2012 at 6:16 PM, Roland Mainz <roland.ma...@nrubsig.org> wrote:
> On Mon, Jul 9, 2012 at 12:59 PM, Lionel Cons
> <lionelcons1...@googlemail.com> 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 ?

----

Bye,
Roland

-- 
  __ .  . __
 (o.\ \/ /.o) roland.ma...@nrubsig.org
  \__\/\/__/  MPEG specialist, C&&JAVA&&Sun&&Unix programmer
  /O /==\ O\  TEL +49 641 3992797
 (;O/ \/ \O;)
diff -r -u original/src/cmd/ksh93/bltins/cd_pwd.c 
build_xattr/src/cmd/ksh93/bltins/cd_pwd.c
--- src/cmd/ksh93/bltins/cd_pwd.c       2012-05-29 15:08:17.000000000 +0200
+++ src/cmd/ksh93/bltins/cd_pwd.c       2012-07-11 03:24:38.323051129 +0200
@@ -19,8 +19,8 @@
 ***********************************************************************/
 #pragma prototyped
 /*
- * cd [-LP]  [dirname]
- * cd [-LP]  [old] [new]
+ * cd [-LP@]  [dirname]
+ * cd [-LP@]  [old] [new]
  * pwd [-LP]
  *
  *   David Korn
@@ -38,6 +38,11 @@
 #include       "builtins.h"
 #include       <ls.h>
 
+/* Repeat syscall in expr each time it gets hit with EINTR */
+#ifndef EINTR_REPEAT
+#define EINTR_REPEAT(expr) while((expr) && (errno == EINTR)) errno=0;
+#endif
+
 /*
  * Invalidate path name bindings to relative paths
  */
@@ -49,6 +54,89 @@
                _nv_unset(np,0);
 }
 
+int sh_diropenat(Shell_t *shp, int dir, const char *path, int xattr)
+{
+       int fd;
+       int shfd;
+       int savederrno;
+
+#ifdef O_XATTR
+       if(xattr)
+       {
+               int apfd; /* attribute parent fd */
+
+               /* open parent node... */
+               EINTR_REPEAT((apfd = openat(dir, path, 
O_RDONLY|O_NONBLOCK|O_cloexec)) < 0);
+               if(apfd < 0)
+                       return -1;
+
+               /* ... and then open a fd to the attribute directory */
+               EINTR_REPEAT((fd = openat(apfd, e_dot, O_XATTR|O_cloexec)) < 0);
+
+               savederrno = errno;
+               EINTR_REPEAT(close(apfd) < 0);
+               errno = savederrno;
+       }
+       else
+#endif
+       {
+#ifdef AT_FDCWD
+               /*
+                * Open directory. First we try without |O_SEARCH| and
+                * if this fails with EACCESS we try with |O_SEARCH|
+                * again.
+                * This is required ...
+                * - ... because some platforms may require that it can
+                * only be used for directories while some filesystems
+                * (e.g. Reiser4 or HSM systems) allow a |fchdir()| into
+                * files, too)
+                * - ... to preserve the semantics of "cd", e.g.
+                * otherwise "cd" would return [No access] instead of
+                * [Not a directory] for files on filesystems which do
+                * not allow a "cd" into files.
+                * - ... to allow that a
+                * $ redirect {n}</etc ; cd /dev/fd/$n # works on most
+                * platforms.
+                */
+               EINTR_REPEAT((fd = openat(dir, path, O_cloexec)) < 0);
+#ifdef O_SEARCH
+               if((fd < 0) && (errno == EACCES))
+               {
+                       EINTR_REPEAT((fd = openat(dir, path, 
O_SEARCH|O_cloexec)) < 0)
+               }
+#endif
+#else
+               /*
+                * Version of openat() call above for systems without
+                * openat API. This only works because we basically
+                * gurantee that |dir| is always the same place as
+                * |cwd| on such machines (but this won't be the case
+                * in the future).
+                */
+               EINTR_REPEAT((fd = open(path, O_cloexec)) < 0);
+#endif
+       }
+
+       if(fd < 0)
+               return fd;
+
+       /* Move fd to a number > 10 and *register* the fd number with the shell 
*/
+       shfd = sh_fcntl(fd, F_DUPFD, 10);
+       savederrno=errno;
+
+       EINTR_REPEAT(close(fd) < 0);
+
+       /*
+        * FIXME: |sh_fcntl()| should implement F_DUPFD_CLOEXEC
+        * and F_DUP2FD_CLOEXEC to reduce the number of system calls
+        */
+       if(shfd >=0)
+               sh_fcntl(shfd, F_SETFD, FD_CLOEXEC);
+       errno=savederrno;
+       
+       return shfd;
+}
+
 int    b_cd(int argc, char *argv[],Shbltin_t *context)
 {
        register char *dir;
@@ -56,8 +144,10 @@
        register const char *dp;
        register Shell_t *shp = context->shp;
        int saverrno=0;
-       int rval,flag=0;
+       int rval,flag=0,xattr=0;
        char *oldpwd;
+       int newdirfd;
+
        Namval_t *opwdnod, *pwdnod;
        if(sh_isoption(SH_RESTRICTED))
                errormsg(SH_DICT,ERROR_exit(1),e_restricted+4);
@@ -69,6 +159,11 @@
                case 'P':
                        flag = 1;
                        break;
+#ifdef O_XATTR
+               case '@':
+                       xattr = 1;
+                       break;
+#endif
                case ':':
                        errormsg(SH_DICT,2, "%s", opt_info.arg);
                        break;
@@ -131,6 +226,15 @@
                        cdpath = 0;
                }
        }
+#ifdef O_XATTR
+       if (xattr)
+#ifdef PATH_BFPATH
+               cdpath = NULL
+#else
+               cdpath = ""
+#endif
+       ;
+#endif
        rval = -1;
        do
        {
@@ -171,15 +275,77 @@
                                        continue;
 #endif /* SHOPT_FS_3D */
                }
-               if((rval=chdir(path_relative(shp,stakptr(PATH_OFFSET)))) >= 0)
-                       goto success;
-               if(errno!=ENOENT && saverrno==0)
+
+               rval = newdirfd = sh_diropenat(shp,
+                       ((shp->pwdfd >= 0)?shp->pwdfd:AT_FDCWD),
+                       path_relative(shp,stakptr(PATH_OFFSET)),
+                       xattr);
+               if(newdirfd >=0)
+               {
+                       /* chdir for directories on HSM/tapeworms may take 
minutes */
+                       EINTR_REPEAT((rval=fchdir(newdirfd)) < 0);
+                       if(rval >= 0)
+                       {
+                               if(shp->pwdfd >= 0)
+                                       sh_close(shp->pwdfd);
+                               shp->pwdfd=newdirfd;
+                               goto success;
+                       }
+               }
+#ifndef O_SEARCH
+               else
+               {
+                       rval=chdir(path_relative(shp,stakptr(PATH_OFFSET)));
+                       if(rval >=0)
+                       {
+                               if(shp->pwdfd >= 0)
+                               {
+                                       sh_close(shp->pwdfd);
+                                       shp->pwdfd=-1;
+                               }
+                       }
+               }
+#endif
+               if(saverrno==0)
                        saverrno=errno;
+               if(newdirfd >=0)
+                       sh_close(newdirfd);
        }
        while(cdpath);
-       if(rval<0 && *dir=='/' && 
*(path_relative(shp,stakptr(PATH_OFFSET)))!='/')
-               rval = chdir(dir);
        /* use absolute chdir() if relative chdir() fails */
+       if(rval<0 && *dir=='/' && 
*(path_relative(shp,stakptr(PATH_OFFSET)))!='/')
+       {
+               rval = newdirfd = sh_diropenat(shp,
+                       ((shp->pwdfd >= 0)?shp->pwdfd:AT_FDCWD),
+                       dir,
+                       xattr);
+               if(newdirfd >=0)
+               {
+                       /* chdir for directories on HSM/tapeworms may take 
minutes */
+                       EINTR_REPEAT((rval=fchdir(newdirfd)) < 0);
+                       if(rval >= 0)
+                       {
+                               if(shp->pwdfd >= 0)
+                                       sh_close(shp->pwdfd);
+                               shp->pwdfd=newdirfd;
+                               goto success;
+                       }
+               }
+#ifndef O_SEARCH
+               else
+               {
+                       rval=chdir(dir);
+                       if(rval >=0)
+                       {
+                               if(shp->pwdfd >= 0)
+                               {
+                                       sh_close(shp->pwdfd);
+                                       shp->pwdfd=-1;
+                               }
+                       }
+               }
+#endif
+       }
        if(rval<0)
        {
                if(saverrno)
diff -r -u original/src/cmd/ksh93/data/builtins.c 
build_xattr/src/cmd/ksh93/data/builtins.c
--- src/cmd/ksh93/data/builtins.c       2012-06-19 10:02:12.000000000 +0200
+++ src/cmd/ksh93/data/builtins.c       2012-07-11 03:24:38.326384665 +0200
@@ -440,7 +440,7 @@
 ;
 
 const char sh_optcd[] =
-"[-1c?\n@(#)$Id: cd (AT&T Research) 1999-06-05 $\n]"
+"[-1c?\n@(#)$Id: cd (AT&T Research) 2012-06-18 $\n]"
 USAGE_LICENSE
 "[+NAME?cd - change working directory ]"
 "[+DESCRIPTION?\bcd\b changes the current working directory of the "
@@ -481,6 +481,10 @@
 "[P?The present working directory is first converted to an absolute pathname "
        "that does not contain symbolic link components and symbolic name "
        "components are expanded in the resulting directory name.]"
+#ifdef O_XATTR
+"[@?Change into the hidden attribute directory of a file or directory.  The "
+       "\bCDPATH\b environment variable is being ignored.]"
+#endif
 "\n"
 "\n[directory]\n"
 "old new\n"
diff -r -u original/src/cmd/ksh93/include/defs.h 
build_xattr/src/cmd/ksh93/include/defs.h
--- src/cmd/ksh93/include/defs.h        2012-06-25 20:47:47.000000000 +0200
+++ src/cmd/ksh93/include/defs.h        2012-07-11 03:24:38.327090868 +0200
@@ -245,6 +245,7 @@
        Sfio_t          **sftable; \
        unsigned char   *fdstatus; \
        const char      *pwd; \
+       int             pwdfd; \
        void            *jmpbuffer; \
        void            *mktype; \
        Sfio_t          *strbuf; \
diff -r -u original/src/cmd/ksh93/sh/init.c build_xattr/src/cmd/ksh93/sh/init.c
--- src/cmd/ksh93/sh/init.c     2012-05-11 19:19:10.000000000 +0200
+++ src/cmd/ksh93/sh/init.c     2012-07-11 03:24:38.328286488 +0200
@@ -1365,6 +1365,12 @@
                }
        }
        sh_ioinit(shp);
+       shp->pwdfd = sh_diropenat(shp, AT_FDCWD, ".", 0);
+#ifdef O_SEARCH
+       /* This should _never_ happen, guranteed by design and goat sacrifice */
+       if(shp->pwdfd < 0)
+               errormsg(SH_DICT,ERROR_exit(1), "Can't obtain directory fd.");
+#endif
        /* initialize signal handling */
        sh_siginit(shp);
        stakinstall(NIL(Stak_t*),nospace);
diff -r -u original/src/cmd/ksh93/sh/subshell.c 
build_xattr/src/cmd/ksh93/sh/subshell.c
--- src/cmd/ksh93/sh/subshell.c 2012-05-30 22:52:44.000000000 +0200
+++ src/cmd/ksh93/sh/subshell.c 2012-07-11 03:43:47.751158180 +0200
@@ -75,6 +75,7 @@
        Sfio_t* saveout;/*saved standard output */
        char            *pwd;   /* present working directory */
        const char      *shpwd; /* saved pointer to sh.pwd */
+       int             shpwdfd;
        void            *jobs;  /* save job info */
        mode_t          mask;   /* saved umask */
        short           tmpfd;  /* saved tmp file descriptor */
@@ -520,9 +521,22 @@
        shp->subshare = comsub==2 ||  (comsub==1 && sh_isoption(SH_SUBSHARE));
        if(comsub)
                shp->comsub = comsub;
+
+       sp->shpwdfd=-1;
        if(!comsub || !shp->subshare)
        {
                sp->shpwd = shp->pwd;
+               sp->shpwdfd=((shp->pwdfd >= 0)
+#ifdef AT_FDCWD
+                       &&(shp->pwdfd!=AT_FDCWD)
+#endif
+                       )?sh_fcntl(shp->pwdfd, F_DUPFD, 10):-1;
+               if(sp->shpwdfd>=0)
+                       sh_fcntl(sp->shpwdfd, F_SETFD, FD_CLOEXEC);
+#ifdef O_SEARCH
+               else
+                       errormsg(SH_DICT,ERROR_exit(1), "Can't obtain directory 
fd.");
+#endif
                sp->pwd = (shp->pwd?strdup(shp->pwd):0);
                sp->mask = shp->mask;
                sh_stats(STAT_SUBSHELL);
@@ -699,7 +713,11 @@
                        Namval_t *pwdnod = sh_scoped(shp,PWDNOD);
                        if(shp->pwd)
                        {
-                               chdir(shp->pwd=sp->pwd);
+                               shp->pwd=sp->pwd;
+#ifndef O_SEARCH
+                               if (sp->shpwdfd < 0)
+                                       chdir(shp->pwd);
+#endif
                                path_newdir(shp,shp->pathlist);
                        }
                        if(nv_isattr(pwdnod,NV_NOFREE))
@@ -724,6 +742,15 @@
                shp->cpipe[1] = sp->cpipe;
                shp->coutpipe = sp->coutpipe;
        }
+       if(sp->shpwdfd >=0)
+       {
+               if(shp->pwdfd >=0)
+                       sh_close(shp->pwdfd);
+               shp->pwdfd=sp->shpwdfd;
+               /* chdir for directories on HSM/tapeworms may take minutes */
+               while((fchdir(shp->pwdfd) < 0) && (errno == EINTR))
+                       errno=0;
+       }
        shp->subshare = sp->subshare;
        shp->comsub = sp->comsub;
        shp->subdup = sp->subdup;
diff -r -u original/src/cmd/ksh93/sh/xec.c build_xattr/src/cmd/ksh93/sh/xec.c
--- src/cmd/ksh93/sh/xec.c      2012-06-26 18:03:05.000000000 +0200
+++ src/cmd/ksh93/sh/xec.c      2012-07-11 03:24:38.331870027 +0200
@@ -1346,7 +1346,7 @@
                                                {
                                                        if(!shp->pwd)
                                                                path_pwd(shp,0);
-                                                       if(shp->pwd)
+                                                       else if(shp->pwd || 
(shp->pwdfd >= 0))
                                                                
stat(".",&statb);
                                                        sfsync(NULL);
                                                        share = 
sfset(sfstdin,SF_SHARE,0);
@@ -1426,13 +1426,26 @@
                                                sh_offstate(SH_NOFORK);
                                        if(!(nv_isattr(np,BLT_ENV)))
                                        {
-                                               if(shp->pwd)
+                                               if(shp->pwd || (shp->pwdfd >= 
0))
                                                {
                                                        struct stat stata;
                                                        stat(".",&stata);
                                                        /* restore directory 
changed */
                                                        
if(statb.st_ino!=stata.st_ino || statb.st_dev!=stata.st_dev)
-                                                               chdir(shp->pwd);
+                                                       {
+                                                               if(shp->pwdfd 
>= 0)
+                                                               {
+                                                                       /* 
chdir for directories on HSM/tapeworms may take minutes */
+                                                                       
while((fchdir(shp->pwdfd) < 0) && (errno == EINTR))
+                                                                               
errno=0;
+                                                               }
+                                                               else
+                                                               {
+                                                                       /* 
chdir for directories on HSM/tapeworms may take minutes */
+                                                                       
while((chdir(shp->pwd) < 0) && (errno == EINTR))
+                                                                               
errno=0;
+                                                               }
+                                                       }
                                                }
                                                sh_offstate(SH_STOPOK);
                                                if(share&SF_SHARE)
Only in original/src/cmd/ksh93/tests: arrays.sh
_______________________________________________
ast-developers mailing list
ast-developers@research.att.com
https://mailman.research.att.com/mailman/listinfo/ast-developers

Reply via email to