From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Thu, 22 Dec 2005 22:59:47 GMT
Subject: Re: [PATCH] Introduce core.sharedrepository
Message-ID: <Pine.LNX.4.63.0512222356540.12681@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <7vaceshi18.fsf@assigned-by-dhcp.cox.net>

Hi,

On Thu, 22 Dec 2005, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > +static int make_group_writable(const char *path)
> > +{
> > +	struct stat st;
> > +
> > +	if (lstat(path, &st) < 0)
> > +		return -1;
> > +	if (st.st_mode & S_IWUSR)
> > +		st.st_mode |= S_IWGRP;
> > +	if (S_ISDIR(st.st_mode))
> > +		st.st_mode |= S_ISGID;
> > +	if (chmod(path, st.st_mode) < 0)
> > +		return -2;
> > +	return 0;
> > +}
> 
> Perhaps g+sx not just g+s on directories.

Okay:

        if (S_ISDIR(st.st_mode))
                st.st_mode |= S_ISGID | S_IXGRP;

> > +
> >  int safe_create_leading_directories(char *path)
> >  {
> >  	char *pos = path;
> > @@ -64,6 +79,10 @@ int safe_create_leading_directories(char
> >  				*pos = '/';
> >  				return -1;
> >  			}
> > +		if (shared_repository && make_group_writable(path)) {
> > +			*pos = '/';
> > +			return -2;
> > +		}
> 
> Wouldn't this safe_create_leading_directories("/pub/git.git/objects")
> try chmod "/pub" and fail?

Yes.

How about an "else" in front of the "if"? Like so:

                if (mkdir(path, 0777) < 0)
                        if (errno != EEXIST) {
                                *pos = '/';
                                return -1;
                        }
                else if (shared_repository && make_group_writable(path)) {
                        *pos = '/';
                        return -2;
                }

You want me to resend the patch?

Ciao,
Dscho