threads / patch / 64104

patch, 4 partsstring-list: allow passing NULL for `get_entry_index`

Subject: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`

## tl;dr

6 messages between Sep 7, 2025 and Sep 15, 2025. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

shejialuo· Sep 7, 2025, 16:40 UTC · lore

Callers of `get_entry_index()` are required to pass a non-NULL `exact_match` parameter to receive information about whether an exact match is found. However, in some cases, callers only need the index position.

Let's allow callers to pass NULL for the `exact_match` parameter when they don't need this information, reducing unnecessary variable declarations in calling code.

Signed-off-by: shejialuo <shejialuo@gmail.com>
---
 string-list.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Show changes to string-list.c +4 −2
diff --git a/string-list.c b/string-list.c
index 343cf1ca90..bf358d1a5c 100644
--- a/string-list.c
+++ b/string-list.c
@@ -29,12 +29,14 @@ static size_t get_entry_index(const struct string_list *list, const char *string
 		else if (compare > 0)
 			left = middle + 1;
 		else {
-			*exact_match = 1;
+			if (exact_match)
+				*exact_match = 1;
 			return middle;
 		}
 	}
 
-	*exact_match = 0;
+	if (exact_match)
+		*exact_match = 0;
 	return right;
 }
 

-- 
2.51.0
Junio C Hamano· Sep 8, 2025, 16:48 UTC · re: shejialuo · lore

Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`

shejialuo <shejialuo@gmail.com> writes:
Show 13 quoted lines
> Callers of `get_entry_index()` are required to pass a non-NULL
> `exact_match` parameter to receive information about whether an exact
> match is found. However, in some cases, callers only need the index
> position.
>
> Let's allow callers to pass NULL for the `exact_match` parameter
> when they don't need this information, reducing unnecessary variable
> declarations in calling code.
>
> Signed-off-by: shejialuo <shejialuo@gmail.com>
> ---
>  string-list.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

I do not quite see the point of adding these conditional assignments to clutter the control flow. What benefit do these callers gain by not having to have a throw-away int variable on the stack and passing its address to the call chain?

Show 20 quoted lines
> diff --git a/string-list.c b/string-list.c
> index 343cf1ca90..bf358d1a5c 100644
> --- a/string-list.c
> +++ b/string-list.c
> @@ -29,12 +29,14 @@ static size_t get_entry_index(const struct string_list *list, const char *string
>  		else if (compare > 0)
>  			left = middle + 1;
>  		else {
> -			*exact_match = 1;
> +			if (exact_match)
> +				*exact_match = 1;
>  			return middle;
>  		}
>  	}
>  
> -	*exact_match = 0;
> +	if (exact_match)
> +		*exact_match = 0;
>  	return right;
>  }
Ben Knoble· Sep 10, 2025, 00:09 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`

Show 22 quoted lines
> Le 8 sept. 2025 à 12:48, Junio C Hamano <gitster@pobox.com> a écrit :
> 
> shejialuo <shejialuo@gmail.com> writes:
> 
>> Callers of `get_entry_index()` are required to pass a non-NULL
>> `exact_match` parameter to receive information about whether an exact
>> match is found. However, in some cases, callers only need the index
>> position.
>> 
>> Let's allow callers to pass NULL for the `exact_match` parameter
>> when they don't need this information, reducing unnecessary variable
>> declarations in calling code.
>> 
>> Signed-off-by: shejialuo <shejialuo@gmail.com>
>> ---
>> string-list.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
> 
> I do not quite see the point of adding these conditional assignments
> to clutter the control flow.  What benefit do these callers gain by
> not having to have a throw-away int variable on the stack and
> passing its address to the call chain?
Wouldn’t the point be that (at the cost of slightly more interesting library code) callers don’t need to introduce extra ceremony for results they don’t use? In other words, take the boilerplate and push it down rather than up?
Junio C Hamano· Sep 10, 2025, 01:32 UTC · re: Ben Knoble · lore

Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`

Ben Knoble <ben.knoble@gmail.com> writes:
Show 24 quoted lines
>> Le 8 sept. 2025 à 12:48, Junio C Hamano <gitster@pobox.com> a écrit :
>> 
>> shejialuo <shejialuo@gmail.com> writes:
>> 
>>> Callers of `get_entry_index()` are required to pass a non-NULL
>>> `exact_match` parameter to receive information about whether an exact
>>> match is found. However, in some cases, callers only need the index
>>> position.
>>> 
>>> Let's allow callers to pass NULL for the `exact_match` parameter
>>> when they don't need this information, reducing unnecessary variable
>>> declarations in calling code.
>>> 
>>> Signed-off-by: shejialuo <shejialuo@gmail.com>
>>> ---
>>> string-list.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> I do not quite see the point of adding these conditional assignments
>> to clutter the control flow.  What benefit do these callers gain by
>> not having to have a throw-away int variable on the stack and
>> passing its address to the call chain?
>
> Wouldn’t the point be that (at the cost of slightly more interesting library code) callers don’t need to introduce extra ceremony for results they don’t use? In other words, take the boilerplate and push it down rather than up?

Yeah, but the point is in what situation does a caller *not* need to be able to tell between "we found an existing one and this is its index" vs "there is no such thing, but here is where it *would* fit if it were placed in the array"? I do not think of any, so I doubted if it makes much sense to complicate the interface to make it optional.

Ben Knoble· Sep 10, 2025, 02:04 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`

Show 36 quoted lines
> Le 9 sept. 2025 à 21:32, Junio C Hamano <gitster@pobox.com> a écrit :
> 
> Ben Knoble <ben.knoble@gmail.com> writes:
> 
>>>> Le 8 sept. 2025 à 12:48, Junio C Hamano <gitster@pobox.com> a écrit :
>>> 
>>> shejialuo <shejialuo@gmail.com> writes:
>>> 
>>>> Callers of `get_entry_index()` are required to pass a non-NULL
>>>> `exact_match` parameter to receive information about whether an exact
>>>> match is found. However, in some cases, callers only need the index
>>>> position.
>>>> 
>>>> Let's allow callers to pass NULL for the `exact_match` parameter
>>>> when they don't need this information, reducing unnecessary variable
>>>> declarations in calling code.
>>>> 
>>>> Signed-off-by: shejialuo <shejialuo@gmail.com>
>>>> ---
>>>> string-list.c | 6 ++++--
>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>> 
>>> I do not quite see the point of adding these conditional assignments
>>> to clutter the control flow.  What benefit do these callers gain by
>>> not having to have a throw-away int variable on the stack and
>>> passing its address to the call chain?
>> 
>> Wouldn’t the point be that (at the cost of slightly more interesting library code) callers don’t need to introduce extra ceremony for results they don’t use? In other words, take the boilerplate and push it down rather than up?
> 
> Yeah, but the point is in what situation does a caller *not* need to
> be able to tell between "we found an existing one and this is its
> index" vs "there is no such thing, but here is where it *would* fit
> if it were placed in the array"?  I do not think of any, so I
> doubted if it makes much sense to complicate the interface to make
> it optional.
> 
Good question! Thanks for clarifying.
shejialuo· Sep 15, 2025, 12:08 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`

On Mon, Sep 08, 2025 at 09:48:12AM -0700, Junio C Hamano wrote:
Show 21 quoted lines
> shejialuo <shejialuo@gmail.com> writes:
> 
> > Callers of `get_entry_index()` are required to pass a non-NULL
> > `exact_match` parameter to receive information about whether an exact
> > match is found. However, in some cases, callers only need the index
> > position.
> >
> > Let's allow callers to pass NULL for the `exact_match` parameter
> > when they don't need this information, reducing unnecessary variable
> > declarations in calling code.
> >
> > Signed-off-by: shejialuo <shejialuo@gmail.com>
> > ---
> >  string-list.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> I do not quite see the point of adding these conditional assignments
> to clutter the control flow.  What benefit do these callers gain by
> not having to have a throw-away int variable on the stack and
> passing its address to the call chain?
>

I want to make the commit small. However, I have introduced confusion to the reviewers. I would rebase the first commit and second commit into one commit to improve this.

Thanks, Jialuo

← back to recent threads