From: Johannes Sixt <johannes.sixt@telecom.at>
Date: Sat, 03 Mar 2007 12:20:13 GMT
Subject: Re: [PATCH] Add core.symlinks to mark filesystems that do not support symbolic links.
Message-ID: <200703031320.13535.johannes.sixt@telecom.at>
In-Reply-To: <7vr6s7chim.fsf@assigned-by-dhcp.cox.net>

On Saturday 03 March 2007 01:12, Junio C Hamano wrote:
> Junio C Hamano <junkio@cox.net> writes:
> > This code (I am the guilty one before your change above) always
> > confused me.  How about doing something like this instead?
> >
> >     static inline unsigned int ce_mode_from_stat(struct cache_entry
> > *ce,... {
> >         /*
> >          * A regular file that appears on the filesystem can have
> >          * a "wrong" st_mode information.  A few repository config
> >          * variables can tell us to trust the mode recorded in the
> >          * index more than what we get from the filesystem.
> >          */
> >         if (ce && S_ISREG(mode)) {
> >             extern int trust_executable_bit, has_symlinks;
> >
> >             if (!has_symlinks && S_ISLNK(ntohl(ce->ce_mode)))
> >                 return ce->ce_mode;
>
> Oops, these three lines
>
> >             if (!trust_executable_bit && S_ISREG(ntohl(ce->ce_mode)))
> >                 return ce->ce_mode;
> >             return create_ce_mode(0666);
>
> should be:
>
> 		if (!trust...) {
>                 	if (S_ISREG(...))
>                         	return ce->ce_mode;
> 			return create_ce_mode(0666);
> 		}

I think that's still not correct. Because in the case of !trust_executable_bit 
we want create_ce_mode(0666) regardless of whether a ce exists or not. Maybe 
this way:

    static inline unsigned int ce_mode_from_stat(struct cache_entry *ce,...
    {
        /*
         * A regular file that appears on the filesystem can have
         * a "wrong" st_mode information.  A few repository config
         * variables can tell us to trust the mode recorded in the
         * index more than what we get from the filesystem.
         */
        if (S_ISREG(mode)) {
            extern int trust_executable_bit, has_symlinks;

            if (!has_symlinks && ce && S_ISLNK(ntohl(ce->ce_mode)))
                return ce->ce_mode;
            if (!trust_executable_bit) {
                if (ce && S_ISREG(ntohl(ce->ce_mode)))
                    return ce->ce_mode;
                return create_ce_mode(0666);
            }
        }
        return create_ce_mode(mode);
    }


-- Hannes