On Wed, Feb 06, 2013 at 04:08:50PM +0100, Michael J Gruber wrote:

> diff --git a/builtin/log.c b/builtin/log.c
> index 8f0b2e8..f83870d 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -402,10 +402,28 @@ static void show_tagger(char *buf, int len, struct 
> rev_info *rev)
>       strbuf_release(&out);
>  }
>  
> -static int show_blob_object(const unsigned char *sha1, struct rev_info *rev)
> +static int show_blob_object(const unsigned char *sha1, struct rev_info *rev, 
> const char *obj_name)

Should this maybe just take the whole object_array_entry as a cleanup?

>  {
> +     unsigned char sha1c[20];
> +     struct object_context obj_context;
> +     char *buf;
> +     unsigned long size;
> +
>       fflush(stdout);
> -     return stream_blob_to_fd(1, sha1, NULL, 0);
> +     if (!DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
> +             return stream_blob_to_fd(1, sha1, NULL, 0);
> +
> +     if (get_sha1_with_context(obj_name, 0, sha1c, &obj_context))
> +             die("Not a valid object name %s", obj_name);

It seems a little hacky that we have to look up the sha1 again. What
should happen in the off chance that "hashcmp(sha1, sha1c) != 0" due to
a race with a simultaneous update of a ref?

Would it be better if object_array_entry replaced its "mode" member with
an object_context? The only downside I see is that we might waste a
significant amount of memory (each context has a PATH_MAX buffer in it).

-Peff
--
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

Reply via email to