2010/6/7 Jeff King <peff@xxxxxxxx>:
> 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
>
Yes, we've already thought about it (you've said it, Jeff ;-) ), and we have
actually changed it with something very close to what you say.
We'll soon propose a v2 for this part of the patch.
Thank you
--
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