[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Make CVS HEAD version build out of the box
From: |
Yongwei Wu |
Subject: |
Re: Make CVS HEAD version build out of the box |
Date: |
Wed, 3 Oct 2007 09:26:46 +0800 |
On 02/10/2007, Eli Zaretskii <address@hidden> wrote:
> > Date: Tue, 2 Oct 2007 19:57:10 +0800
> > From: "Yongwei Wu" <address@hidden>
> > Cc: address@hidden
> >
> > 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/
- Re: Make CVS HEAD version build out of the box, (continued)
- Re: Make CVS HEAD version build out of the box, Eli Zaretskii, 2007/10/09
- Re: Make CVS HEAD version build out of the box, Paul Smith, 2007/10/09
- Re: Make CVS HEAD version build out of the box, Yongwei Wu, 2007/10/09
- Re: Make CVS HEAD version build out of the box, Paul Smith, 2007/10/10
- Re: Make CVS HEAD version build out of the box, Eli Zaretskii, 2007/10/10
- Re: Make CVS HEAD version build out of the box, grischka, 2007/10/01
- Re: Make CVS HEAD version build out of the box, Eli Zaretskii, 2007/10/01
- Re: Make CVS HEAD version build out of the box, Eli Zaretskii, 2007/10/01
- Re: Make CVS HEAD version build out of the box, Yongwei Wu, 2007/10/02
- Re: Make CVS HEAD version build out of the box, Eli Zaretskii, 2007/10/02
- Re: Make CVS HEAD version build out of the box,
Yongwei Wu <=
- Re: Make CVS HEAD version build out of the box, Eli Zaretskii, 2007/10/03
- Re: Make CVS HEAD version build out of the box, Paul Smith, 2007/10/02
- Re: Make CVS HEAD version build out of the box, Eli Zaretskii, 2007/10/03
- Re: Make CVS HEAD version build out of the box, Yongwei Wu, 2007/10/03
- Re: Make CVS HEAD version build out of the box, Eli Zaretskii, 2007/10/03
- Re: Make CVS HEAD version build out of the box, Earnie Boyd, 2007/10/03
- Re: Make CVS HEAD version build out of the box, Eli Zaretskii, 2007/10/03
- Re: Make CVS HEAD version build out of the box, Paul Smith, 2007/10/03
- Re: Make CVS HEAD version build out of the box, Yongwei Wu, 2007/10/03
- RE: Make CVS HEAD version build out of the box, Dave Korn, 2007/10/03