Hi Junio,

On 2015-06-19 21:03, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schinde...@gmx.de> writes:
> 
>> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
>> index 48fa472..87ae9ba 100644
>> --- a/builtin/index-pack.c
>> +++ b/builtin/index-pack.c
>> @@ -75,6 +75,7 @@ static int nr_threads;
>>  static int from_stdin;
>>  static int strict;
>>  static int do_fsck_object;
>> +static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
> 
> So there is a global fsck_options used throughout the entire
> session here.
> 
>> @@ -838,10 +839,10 @@ static void sha1_object(const void *data, struct 
>> object_entry *obj_entry,
>>                      if (!obj)
>>                              die(_("invalid %s"), typename(type));
>>                      if (do_fsck_object &&
>> -                        fsck_object(obj, buf, size, 1,
>> -                                fsck_error_function))
>> +                        fsck_object(obj, buf, size, &fsck_options))
>>                              die(_("Error in object"));
> 
> And that is used here to inspect each and every object we encounter.
> 
>> -                    if (fsck_walk(obj, mark_link, NULL))
>> +                    fsck_options.walk = mark_link;
> 
> Then we do a call to fsck_walk() starting from this object, letting
> mark_link() to inspect it and set the LINK bit.
> 
>> +                    if (fsck_walk(obj, NULL, &fsck_options))
>>                              die(_("Not all child objects of %s are 
>> reachable"), sha1_to_hex(obj->sha1));
> 
> Since nobody else sets fsck_options.walk to any other value, and
> nobody else calls fsck_walk(), shouldn't that assignment be done
> only once somewhere a lot higher in the callchain?  The apparent
> "overriding while inspecting this object" that does not have any
> corresponding "now we are done, so revert it to the original value"
> puzzled me, and I am sure it would puzzle future readers of this
> code.

Good point. I guess I was really wary that a configured walk function might 
change the behavior of `fsck_object()`. But after inspecting the code paths 
carefully, I conclude that the walk function is really only used in the 
`fsck_walk_*()` family of functions.

I changed that locally, will be part of v7.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in

Reply via email to