X-Authentication-Warning: delorie.com: mail set sender to djgpp-workers-bounces using -f
X-Recipient: djgpp-workers AT delorie DOT com
Date: Tue, 22 Apr 2014 22:10:24 +0300
From: Eli Zaretskii <eliz AT gnu DOT org>
Subject: Re: [PATCH] break xstat.c into pieces
In-reply-to: <CAA2C=vD_97x3-A0Y52JDfbmmFiuVzF_Ufub6imhmuhTsdsNr4w@mail.gmail.com>
X-012-Sender: halo1 AT inter DOT net DOT il
To: djgpp-workers AT delorie DOT com
Message-id: <83vbu19o4f.fsf@gnu.org>
References: <CAA2C=vBuWHmVNrRAcGDSXYLc8Eg_xsyMVszPzrxqHGrpVq2vmw AT mail DOT gmail DOT com> <83wqeh9pwq DOT fsf AT gnu DOT org> <CAA2C=vD_97x3-A0Y52JDfbmmFiuVzF_Ufub6imhmuhTsdsNr4w AT mail DOT gmail DOT com>
Reply-To: djgpp-workers AT delorie DOT com
Errors-To: nobody AT delorie DOT com
X-Mailing-List: djgpp-workers AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk

> Date: Tue, 22 Apr 2014 21:54:16 +0300
> From: Ozkan Sezer <sezeroz AT gmail DOT com>
> 
> On 4/22/14, Eli Zaretskii <eliz AT gnu DOT org> wrote:
> >> Date: Tue, 22 Apr 2014 14:11:36 +0300
> >> From: Ozkan Sezer <sezeroz AT gmail DOT com>
> >>
> >> The following patch breaks xstat.c into smaller pieces, so that, e.g.
> >> mkdir() doesn't pull in unnecessary dependencies such as ctime.o.
> >
> > I'd rather change 'mkdir' so that it doesn't call 'access', but only
> > its small subset that is needed for testing the D_OK flag.  That
> > sounds like a smaller and more localized change to me.  WDYT?
> >
> 
> Changing mkdir() is the first thing that conmes to mind, but there
> is also _is_executable(), which is a public func through sys/stat.h,
> an it relies on _djstat_flags which is in xstat.c at present.

_is_executable is itself quite a monster, so I'm not sure adding ctime
to it matters.  But if it does (I'd like to hear arguments to that
effect), just pull _djstat_flags out of it and into, say, is_exec.c,
and that should do the trick, I believe.

> I suggest that we chop xstat.c into pieces _and_ change mkdir() too.

I see no need to chop, just get _djstat_flags out of there.

> A simple patch for mkdir() itself is inlined below. Note, however,
> this eliminates the _fixpath() operation present in access(): I do
> not know how necessary that is.

It seems to be necessary for root directories, so I'd keep it.

> diff -u -p -r1.7 mkdir.c
> --- mkdir.c	2 Oct 2011 02:40:11 -0000	1.7
> +++ mkdir.c	22 Apr 2014 18:50:40 -0000
> @@ -73,7 +73,8 @@ do_mkdir:
>      {
>        /* see if the directory existed, in which case
>           we should return EEXIST - DJ */
> -      if (access(mydirname, D_OK) == 0)
> +      if ((attr = _chmod(dir_name, 0)) != -1 &&
> +           attr & 0x10)

Not enough: _chmod doesn't resolve symlinks (assuming we want to
return EEXIST for symlinks to directories, that is -- what does Posix
say?).