* [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`
@ 2025-09-07 16:40 shejialuo
2025-09-08 16:48 ` Junio C Hamano
0 siblings, 1 reply; 8+ messages in thread
From: shejialuo @ 2025-09-07 16:40 UTC (permalink / raw)
To: git
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(-)
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`
2025-09-07 16:40 [PATCH 0/4] enhance string-list API to fix sign compare warnings shejialuo
@ 2025-09-07 16:42 ` shejialuo
2025-09-09 6:22 ` Patrick Steinhardt
0 siblings, 1 reply; 8+ messages in thread
From: shejialuo @ 2025-09-07 16:42 UTC (permalink / raw)
To: git
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(-)
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`
2025-09-07 16:40 [PATCH 1/4] string-list: allow passing NULL for `get_entry_index` shejialuo
@ 2025-09-08 16:48 ` Junio C Hamano
2025-09-10 0:09 ` Ben Knoble
2025-09-15 12:08 ` shejialuo
0 siblings, 2 replies; 8+ messages in thread
From: Junio C Hamano @ 2025-09-08 16:48 UTC (permalink / raw)
To: shejialuo; +Cc: git
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?
> 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;
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`
2025-09-07 16:42 ` [PATCH 1/4] string-list: allow passing NULL for `get_entry_index` shejialuo
@ 2025-09-09 6:22 ` Patrick Steinhardt
0 siblings, 0 replies; 8+ messages in thread
From: Patrick Steinhardt @ 2025-09-09 6:22 UTC (permalink / raw)
To: shejialuo; +Cc: git
On Mon, Sep 08, 2025 at 12:42:20AM +0800, shejialuo wrote:
> 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.
It would be nice to either explain why we want this or, alternatively,
to include that specific user in the same patch.
Patrick
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`
2025-09-08 16:48 ` Junio C Hamano
@ 2025-09-10 0:09 ` Ben Knoble
2025-09-10 1:32 ` Junio C Hamano
2025-09-15 12:08 ` shejialuo
1 sibling, 1 reply; 8+ messages in thread
From: Ben Knoble @ 2025-09-10 0:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: shejialuo, git
> 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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`
2025-09-10 0:09 ` Ben Knoble
@ 2025-09-10 1:32 ` Junio C Hamano
2025-09-10 2:04 ` Ben Knoble
0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2025-09-10 1:32 UTC (permalink / raw)
To: Ben Knoble; +Cc: shejialuo, git
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`
2025-09-10 1:32 ` Junio C Hamano
@ 2025-09-10 2:04 ` Ben Knoble
0 siblings, 0 replies; 8+ messages in thread
From: Ben Knoble @ 2025-09-10 2:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: shejialuo, git
> 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`
2025-09-08 16:48 ` Junio C Hamano
2025-09-10 0:09 ` Ben Knoble
@ 2025-09-15 12:08 ` shejialuo
1 sibling, 0 replies; 8+ messages in thread
From: shejialuo @ 2025-09-15 12:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Mon, Sep 08, 2025 at 09:48:12AM -0700, Junio C Hamano wrote:
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-15 12:08 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-07 16:40 [PATCH 1/4] string-list: allow passing NULL for `get_entry_index` shejialuo
2025-09-08 16:48 ` Junio C Hamano
2025-09-10 0:09 ` Ben Knoble
2025-09-10 1:32 ` Junio C Hamano
2025-09-10 2:04 ` Ben Knoble
2025-09-15 12:08 ` shejialuo
-- strict thread matches above, loose matches on Subject: below --
2025-09-07 16:40 [PATCH 0/4] enhance string-list API to fix sign compare warnings shejialuo
2025-09-07 16:42 ` [PATCH 1/4] string-list: allow passing NULL for `get_entry_index` shejialuo
2025-09-09 6:22 ` Patrick Steinhardt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).