Junio C Hamano <gits...@pobox.com> writes:
> Gabriel Souza Franco <gabrielfrancoso...@gmail.com> writes:
>
>> Check was introduced in b791642 (filter_ref: avoid overwriting
>> ref->old_sha1 with garbage, 2015-03-19), but was always false because
>> ref->old_oid.hash is empty in this case. Instead copy sha1 from ref->name.
>>
>> Signed-off-by: Gabriel Souza Franco <gabrielfrancoso...@gmail.com>
>> ---
>
> Peff, that commit points me at your direction. And I can see the
> original patch avoids overwriting old_sha1 by saving the result from
> get_sha1_hex() in a temporary, it is true that old_sha1 is not
> updated from the temporary.
>
> The original code before b791642 wanted to say "if ref->name is not
> 40-hex, continue, and otherwise, do the ref->matched thing" and an
> implementation of b791642 that is more faithful to the original
> would indeed have been the result of applying this patch from
> Gabriel, but I am scratching my head why we have hashcmp() there.
>
> Was it to avoid adding the same thing twice to the resulting list,
> or something?
Nah, I think you just misspelt hashcpy() as hashcmp(). The original
wanted to get the binary representation of the hex in old_sha1 when
it continued, and you wanted to delay the touching of old_sha1 until
we make sure that the input is valid 40-hex, so something like this
is what Gabriel wants to do (which I agree with), isn't it?
fetch-pack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 058c258..bb5237f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -551,7 +551,7 @@ static void filter_refs(struct fetch_pack_args *args,
continue;
if (get_sha1_hex(ref->name, sha1) ||
ref->name[40] != '\0' ||
- hashcmp(sha1, ref->old_sha1))
+ hashcpy(ref->old_sha1, sha1))
continue;
ref->matched = 1;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
