Re: [PATCH 3/3] RFC: blame,cat-file --textconv: Don't assume mode is ``S_IFREF | 0664''

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Sep 18, 2010, 19:04 UTC
Message-ID: <vpqmxrevppb.fsf@bauges.imag.fr>
In-Reply-To: <7ab60e32582447ad792602ff405dcee464ef1414.1284830388.git.kirr@landau.phys.spbu.ru>

Kirill Smelkov <kirr@landau.phys.spbu.ru> writes:

> I don't know blame code well, and I'm not sure I'm doing it right or
> otherwise without mistakes. Thus an RFC.

I don't know the code well either, but I didn't see any obvious problem in your code.

Show 8 quoted lines

> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -1068,7 +1068,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
>  		struct cache_entry *ce;
>  		int pos;
>  		if (namelen > 2 && name[1] == '/')
> -			return get_sha1_oneline(name + 2, sha1);
> +			return get_sha1_oneline(name + 2, sha1);    /* XXX also mode? */

(This is the case where we're parsing ":/foo")

Currently, the mode is set a few lines above:

	oc->mode = S_IFINVALID;

I guess this is OK since :/foo will return a commit sha1, not a file sha1.

Show 5 quoted lines

> @@ -1095,6 +1095,7 @@ int get_sha1_with_context_1(const char *name, unsigned char *sha1,
>  				break;
>  			if (ce_stage(ce) == stage) {
>  				hashcpy(sha1, ce->sha1);
> +				oc->mode = ce->ce_mode; /* XXX ok? */

I'd say this is OK, you're setting the mode from the index. What was the reason for your question mark?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/