Re: [PATCH v6 01/19] fsck: Introduce fsck options

> Johannes Schindelin <johannes.schindelin@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.