Re: [PATCH 05/12] process_tree/blob: check for NULL

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 19, 2008, 21:31 UTC
Message-ID: <7vodaciqg5.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <12033676833341-git-send-email-mkoegler@auto.tuwien.ac.at>

Martin Koegler <mkoegler@auto.tuwien.ac.at> writes:

Show 30 quoted lines

> As these functions are directly called with the result
> from lookup_tree/blob, they must handle NULL.
>
> Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>
> ---
>  list-objects.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/list-objects.c b/list-objects.c
> index 4ef58e7..c8b8375 100644
> --- a/list-objects.c
> +++ b/list-objects.c
> @@ -18,6 +18,8 @@ static void process_blob(struct rev_info *revs,
>  
>  	if (!revs->blob_objects)
>  		return;
> +	if (!obj)
> +		die("bad blob object");
>  	if (obj->flags & (UNINTERESTING | SEEN))
>  		return;
>  	obj->flags |= SEEN;
> @@ -69,6 +71,8 @@ static void process_tree(struct rev_info *revs,
>  
>  	if (!revs->tree_objects)
>  		return;
> +	if (!obj)
> +		die("bad tree object");
>  	if (obj->flags & (UNINTERESTING | SEEN))
>  		return;
>  	if (parse_tree(tree) < 0)

I think these are in line with process_tree() that barfs like this:

	if (parse_tree(tree) < 0)
		die("bad tree object %s", sha1_to_hex(obj->sha1));

in the existing codepath, but these new die() callsites lose information.

It would be nicer if we can report what entry (name) in which tree object (sha1) lead to this die(). The same comment applies to [09/12].

Nevertheless, this is an improvement compared to accessing NULL->flags and dying with segv. I'll queue.

Thanks.