On Sun, Jun 06, 2010 at 05:56:41PM +0200, Matthieu Moy wrote:
> > +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?
This is straight from my earlier patch, but I think I said in that
thread that it could use some cleaning up. You added the 'mode' field,
but it probably makes sense to then get rid of the distinction between
get_sha1 and get_sha1_with_mode, and just have:
get_sha1_with_context(const char *name, unsigned char *sha1, struct
object_resolve_context *orc) {
/* basically get_sha1_with_mode, but set mode in orc */
}
get_sha1(const char *name, unsigned char *sha1) {
struct object_resolve_context orc; /* to be thrown away */
return get_sha1_with_context(name, sha1, &orc);
}
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html