Re: [RFC/PATCH 1/4] sha1_name: creating context cache

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Clément Poulain <clement.poulain@ensimag.imag.fr> writes:

> This cache keeps the global context for the last sha1 looked up, especially its pathname. The textconv is indeed defined by the diff driver, which is associated with a pathname, not a blob

Please, wrap your message to make it fit in 80 characters (M-q under
Emacs can help). This applies to the cover email too, but is
particularly important for this one, since it'll become the commit
message if the patch is accepted.

> diff --git a/cache.h b/cache.h
> index 0f4263c..26a1faf 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -730,6 +730,14 @@ static inline unsigned int hexval(unsigned char c)
>  #define MINIMUM_ABBREV 4
>  #define DEFAULT_ABBREV 7
>  
> +struct object_resolve_context {
> +	unsigned char tree[20];
> +	char path[PATH_MAX];
> +	unsigned mode;
> +};
> +extern struct object_resolve_context object_context;

Is it really a good idea to make this a global variable? As I
understand it, the semantics of this variable is that it contains
information on the last sha1 name parsed (BTW, you probably want to
add a comment here explaining that). Wouldn't it be more robust to
have this value returned by the function doing the parsing?

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