Re: [PATCH v2] sha1_name: fix uninitialized memory errors

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 28, 2018, 20:50 UTC
Message-ID: <xmqqy3jckfe3.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180227114704.193145-1-dstolee@microsoft.com>

Derrick Stolee <dstolee@microsoft.com> writes:

Show 26 quoted lines

> diff --git a/sha1_name.c b/sha1_name.c
> index 611c7d24dd..a041d8d24f 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -547,15 +547,15 @@ static void find_abbrev_len_for_pack(struct packed_git *p,
>  	 */
>  	mad->init_len = 0;
>  	if (!match) {
> -		nth_packed_object_oid(&oid, p, first);
> -		extend_abbrev_len(&oid, mad);
> +		if (nth_packed_object_oid(&oid, p, first))
> +			extend_abbrev_len(&oid, mad);
>  	} else if (first < num - 1) {
> -		nth_packed_object_oid(&oid, p, first + 1);
> -		extend_abbrev_len(&oid, mad);
> +		if (nth_packed_object_oid(&oid, p, first + 1))
> +			extend_abbrev_len(&oid, mad);
>  	}
>  	if (first > 0) {
> -		nth_packed_object_oid(&oid, p, first - 1);
> -		extend_abbrev_len(&oid, mad);
> +		if (nth_packed_object_oid(&oid, p, first - 1))
> +			extend_abbrev_len(&oid, mad);
>  	}
>  	mad->init_len = mad->cur_len;
>  }

I do not think they are wrong, but aren't the latter two somewhat redundant? "num" is p->num_objects, and we call (first+1)th element only after we see (first < num - 1), i.e. first+1 < num, and the access to (first-1)th is done only when first > 0. The first one, i.e. when first points at where we _would_ find it if it existed, can access "first" that could be p->num_objects, so the change there makes sense, though.