Re: [PATCH] fsck: check results of lookup_blob() and lookup_tree() for NULL

["René Scharfe" <l.s.r@web.de>]

Am 04.10.2017 um 06:00 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>> lookup_blob() and lookup_tree() can return NULL if they find an object
>> of an unexpected type.  Error out of fsck_walk_tree() in that case, like
>> we do when encountering a bad file mode.  An error message is already
>> shown by object_as_type(), which gets called by the lookup functions.
> 
> The result from options->walk() is checked, and among the callbacks
> that are assigned to the .walk field:
> 
>   - mark_object() in builtin/fsck.c gives its own error message to diagnose broken link
>     and returns 1;
> 
>   - mark_used() in builtin/fsck.c silently returns 1;
> 
>   - mark_link() in builtin/index-pack.c does the same; and
> 
>   - check_object() in builtin/unpack-objects.c does the same,
> 
> when they see a NULL object.
> 
> This patch may avoid the "unexpected behaviour" coming from
> expecting that &((struct tree *)NULL)->object == NULL the current
> code does, but it also changes the behaviour.  The loop used to
> diagnose a fishy entry in the tree we are walking, and kept checking
> the remaining entries in the tree.  You now immediately return, not
> seeing if the later entries in the tree are good and losing objects
> that are referenced by these entries as dangling.
> 
> I am not sure if this is a good change.  I suspect that the "bad
> mode" handling should be made less severe instead.

Makes sense.  Replacement patch coming up.  I'll pass on the mode
handling change, though (at least for now).

René