From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28595 invoked from network); 24 Mar 2009 12:46:35 -0000 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by ns1.primenet.com.au with SMTP; 24 Mar 2009 12:46:35 -0000 Received-SPF: none (ns1.primenet.com.au: domain at sunsite.dk does not designate permitted sender hosts) Received: (qmail 14704 invoked from network); 24 Mar 2009 12:46:30 -0000 Received: from sunsite.dk (130.225.247.90) by a.mx.sunsite.dk with SMTP; 24 Mar 2009 12:46:30 -0000 Received: (qmail 11264 invoked by alias); 24 Mar 2009 12:46:24 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 26772 Received: (qmail 11250 invoked from network); 24 Mar 2009 12:46:23 -0000 Received: from bifrost.dotsrc.org (130.225.254.106) by sunsite.dk with SMTP; 24 Mar 2009 12:46:23 -0000 Received: from cluster-g.mailcontrol.com (cluster-g.mailcontrol.com [208.87.233.190]) by bifrost.dotsrc.org (Postfix) with ESMTPS id 9964F80590EB for ; Tue, 24 Mar 2009 13:46:19 +0100 (CET) Received: from cameurexb01.EUROPE.ROOT.PRI ([193.128.72.68]) by rly23g.srv.mailcontrol.com (MailControl) with ESMTP id n2OCkC3L004688 for ; Tue, 24 Mar 2009 12:46:16 GMT Received: from news01 ([10.99.50.25]) by cameurexb01.EUROPE.ROOT.PRI with Microsoft SMTPSVC(6.0.3790.3959); Tue, 24 Mar 2009 12:46:12 +0000 Date: Tue, 24 Mar 2009 12:46:12 +0000 From: Peter Stephenson To: zsh-workers Subject: Re: cd -s symlink hangs (sometimes?) Message-ID: <20090324124612.26017e86@news01> In-Reply-To: <20090323122714.3373526a@news01> References: <237967ef0903201412h2a7b99c9ya5101509a3972313@mail.gmail.com> <20090320224856.73dae001@pws-pc> <237967ef0903201615x72769fe4va86273c3fa07cb2e@mail.gmail.com> <20090322125410.66a9d294@pws-pc> <237967ef0903221605h11983bb4v4eda8d2a1c41a1c9@mail.gmail.com> <20090323104928.0c59c30f@news01> <237967ef0903230446u6810c06cs511fddcc21fd2a8a@mail.gmail.com> <20090323122714.3373526a@news01> Organization: CSR X-Mailer: Claws Mail 3.5.0 (GTK+ 2.12.8; i386-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 24 Mar 2009 12:46:12.0519 (UTC) FILETIME=[83487370:01C9AC7E] X-Scanned-By: MailControl A_08_51_00 (www.mailcontrol.com) on 10.71.0.133 X-Virus-Scanned: ClamAV 0.92.1/9156/Tue Mar 24 05:11:32 2009 on bifrost X-Virus-Status: Clean On Mon, 23 Mar 2009 12:27:14 +0000 Peter Stephenson wrote: > On Mon, 23 Mar 2009 12:46:10 +0100 > Mikael Magnusson wrote: > > 2009/3/23 Peter Stephenson : > > Trying the patch now and it does stop the leak... but you didn't think > > this adventure was over yet, did you? > > No, I definitely want to fix the diagnostics and at the least the internal > setting of pwd when a cd fails, but I'm not sure what a neat way is. On top of this, I've found another pre-existing bug down to poorly thought out code structure, which may be something to do with the side effect Mikael was seeing: on failure, restoredir() could call upchdir() repeatedly because it thought it was doing a recursive glob since the "level" element wasn't initialised. I only got this sometimes with an optimised compilation; before, I was using a debug build which didn't show it up at all. I've now put the initialisation for a "struct dirsav" in a subroutine. For the diagnostics and pwd, I've just borrowed what the recursive handling in zsh/files does, which is cd to /, set pwd consistently, and report the error. This is at least much better; an algorithm for fixing up the current directory even better is a good deal more complicated and given you can't cd to the correct directory in this case it's not clear how useful it is. So I'm tempted to leave it at this. In addition to "cd -s", this may address theoretical problems with recursive globs under similar circumstances. Index: Src/glob.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/glob.c,v retrieving revision 1.69 diff -u -r1.69 glob.c --- Src/glob.c 27 Jan 2009 09:55:34 -0000 1.69 +++ Src/glob.c 24 Mar 2009 12:37:12 -0000 @@ -492,9 +492,7 @@ int errssofar = errsfound; struct dirsav ds; - ds.ino = ds.dev = 0; - ds.dirname = NULL; - ds.dirfd = ds.level = -1; + init_dirsav(&ds); if (!q) return; Index: Src/utils.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/utils.c,v retrieving revision 1.218 diff -u -r1.218 utils.c --- Src/utils.c 24 Mar 2009 12:14:36 -0000 1.218 +++ Src/utils.c 24 Mar 2009 12:37:12 -0000 @@ -5356,6 +5356,21 @@ return 0; } +/* + * Initialize a "struct dirsav". + * The structure will be set to the directory we want to save + * the first time we change to a different directory. + */ + +/**/ +mod_export void +init_dirsav(Dirsav d) +{ + d->ino = d->dev = 0; + d->dirname = NULL; + d->dirfd = d->level = -1; +} + /* Change directory, without following symlinks. Returns 0 on success, -1 * * on failure. Sets errno to ENOTDIR if any symlinks are encountered. If * * fchdir() fails, or the current directory is unreadable, we might end up * @@ -5379,9 +5394,7 @@ #endif if (!d) { - ds.ino = ds.dev = 0; - ds.dirname = NULL; - ds.dirfd = -1; + init_dirsav(&ds); d = &ds; } #ifdef HAVE_LSTAT @@ -5479,6 +5492,17 @@ } } if (restoredir(d)) { + int restoreerr = errno; + /* + * Failed to restore the directory. + * Just be definite, cd to root and report the result. + */ + zsfree(pwd); + pwd = ztrdup("/"); + if (chdir(pwd) < 0) + zerr("lost current directory, failed to cd to /: %e", errno); + else + zerr("lost current directory: %e: changed to /", restoreerr); if (d == &ds) zsfree(ds.dirname); #ifdef HAVE_FCHDIR Index: Src/zsh.h =================================================================== RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v retrieving revision 1.154 diff -u -r1.154 zsh.h --- Src/zsh.h 3 Mar 2009 17:26:08 -0000 1.154 +++ Src/zsh.h 24 Mar 2009 12:37:12 -0000 @@ -382,6 +382,7 @@ typedef struct cmdnam *Cmdnam; typedef struct complist *Complist; typedef struct conddef *Conddef; +typedef struct dirsav *Dirsav; typedef struct features *Features; typedef struct feature_enables *Feature_enables; typedef struct funcstack *Funcstack; Index: Src/Modules/files.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/Modules/files.c,v retrieving revision 1.20 diff -u -r1.20 files.c --- Src/Modules/files.c 15 Mar 2009 01:04:50 -0000 1.20 +++ Src/Modules/files.c 24 Mar 2009 12:37:12 -0000 @@ -382,9 +382,7 @@ reccmd.dirpost_func = dirpost_func; reccmd.leaf_func = leaf_func; reccmd.magic = magic; - ds.ino = ds.dev = 0; - ds.dirname = NULL; - ds.dirfd = ds.level = -1; + init_dirsav(&ds); if (opt_recurse || opt_safe) { if ((ds.dirfd = open(".", O_RDONLY|O_NOCTTY)) < 0 && zgetdir(&ds) && *ds.dirname != '/') @@ -476,9 +474,7 @@ } err = err1; - dsav.ino = dsav.dev = 0; - dsav.dirname = NULL; - dsav.dirfd = dsav.level = -1; + init_dirsav(&dsav); d = opendir("."); if(!d) { if(!reccmd->opt_noerr) -- Peter Stephenson Software Engineer CSR PLC, Churchill House, Cambridge Business Park, Cowley Road Cambridge, CB4 0WZ, UK Tel: +44 (0)1223 692070