On 02/10/2007, Eli Zaretskii <[EMAIL PROTECTED]> wrote: > > Date: Tue, 2 Oct 2007 19:57:10 +0800 > > From: "Yongwei Wu" <[EMAIL PROTECTED]> > > Cc: make-w32@gnu.org > > > > The patch itself is good. Can I suggest a small alternative? > > > > --- dir.c.bak 2007-10-02 19:33:52.281250000 +0800 > > +++ dir.c 2007-10-02 19:35:35.656250000 +0800 > > @@ -457,10 +457,11 @@ > > r = vmsstat_dir (name, &st); > > #elif defined(WINDOWS32) > > { > > - char tem[MAXPATHLEN], *t; > > + char *tem, *t; > > > > /* Remove any trailing slashes. Windows32 stat fails even on > > valid directories if they end in a slash. */ > > + tem = alloca (p - name + 1); > > memcpy (tem, name, p - name + 1); > > t = tem + (p - name - 1); > > if (*t == '/' || *t == '\\') > > Thanks, but can you explain why alloca is better here? We are talking > about a variable that is local to a small block, and goes out of > existence immediately after that block is exited. What advantage is > there in using alloca? I know about a disadvantage: alloca might have > portability problems to some Windows compilers.
Just a personal preference. I do not like big fixed-size arrays, which look to me both a waste of stack space and a potential source for buffer overrun. Probably not in this case, but I still do not like them. I do not know any decent Windows compilers that do not support alloca. I tested with GCC, MSVC, and Digital Mars. I think Borland C Compiler will do too, but I do not have it at hand. > Btw, I just realized that there could be in principle more than one > trailing slash there, so there needs to be a loop, and it should not > mishandle the case of the root directory. > > A revised patch attached. > > > --- dir.c~0 2007-07-21 19:06:58.251125000 +0300 > +++ dir.c 2007-10-02 17:19:50.187500000 +0200 > @@ -453,27 +453,29 @@ find_directory (const char *name) > hash_insert_at (&directories, dir, dir_slot); > /* The directory is not in the name hash table. > Find its device and inode numbers, and look it up by them. */ > - > -#ifdef WINDOWS32 > - /* Remove any trailing '\'. Windows32 stat fails even on valid > - directories if they end in '\'. */ > - if (p[-1] == '\\') > - p[-1] = '\0'; > -#endif > - > #ifdef VMS > r = vmsstat_dir (name, &st); > +#elif defined(WINDOWS32) > + { > + char tem[MAXPATHLEN], *tstart, *tend; > + > + /* Remove any trailing slashes. Windows32 stat fails even on > + valid directories if they end in a slash. */ > + memcpy (tem, name, p - name + 1); > + tstart = tem; > + if (tstart[1] == ':') > + tstart += 2; > + for (tend = tem + (p - name - 1); > + tend > tstart && (*tend == '/' || *tend == '\\'); > + tend--) > + *tend = '\0'; > + > + r = stat (tem, &st); > + } > #else > EINTRLOOP (r, stat (name, &st)); > #endif > > -#ifdef WINDOWS32 > - /* Put back the trailing '\'. If we don't, we're permanently > - truncating the value! */ > - if (p[-1] == '\0') > - p[-1] = '\\'; > -#endif > - > if (r < 0) > { > /* Couldn't stat the directory. Mark this by You are very careful. Thanks. I have no objections. Just that I never found the "= '\0'" line here being run :-(, even when I have a rule like "test.exe: BUG/". So the copying looks a waste of CPU time. Though I do not believe it can really hurt the performance, I am not comfortable with the facts that there is an extra overhead on Windows and that it has no benefits I can see. Best regards, Yongwei -- Wu Yongwei URL: http://wyw.dcweb.cn/ _______________________________________________ Make-w32 mailing list Make-w32@gnu.org http://lists.gnu.org/mailman/listinfo/make-w32