Re: [PATCH v2 1/3] packfile-uris: support for excluding commit object

From: Junio C Hamano <gitster@pobox.com>
Date: May 20, 2021, 04:46 UTC
Message-ID: <xmqqlf8aui5y.fsf@gitster.g>
In-Reply-To: <73e64147b17cb382d34357c913616095b6169650.1621327467.git.dyroneteng@gmail.com>

(continuing from yesterday's review)

Show 18 quoted lines

> @@ -3023,7 +3040,7 @@ static int add_object_entry_from_pack(const struct object_id *oid,
>  	struct rev_info *revs = _data;
>  	struct object_info oi = OBJECT_INFO_INIT;
>  	off_t ofs;
> -	enum object_type type;
> +	static enum object_type type;
>  
>  	display_progress(progress_state, ++nr_seen);
>  
> @@ -3031,7 +3048,7 @@ static int add_object_entry_from_pack(const struct object_id *oid,
>  		return 0;
>  
>  	ofs = nth_packed_object_offset(p, pos);
> -	if (!want_object_in_pack(oid, 0, &p, &ofs))
> +	if (!want_object_in_pack(oid, type, 0, &p, &ofs))
>  		return 0;
>  
>  	oi.typep = &type;

This change is puzzling.

The first call to this helper will use BSS initialized type to call want_object_in_pack(), and then after that call says "yes, we want that object", packed_object_info() gets called to learn what type it is. And the second call would use the type of the object we previously handled, because type is a function scope static. Or if we are not that lucky and multiple threads are involved, the type we pass is from a random and unrelated object some other thread has called this helper with.