{"thread":{"id":"64104","subject":"[PATCH 1/4] string-list: allow passing NULL for `get_entry_index`","startedAt":"2025-09-07T16:40:28Z","lastAt":"2025-09-15T12:08:05Z","messageCount":6,"participants":["shejialuo","Junio C Hamano","Ben Knoble"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"525713","messageId":"aL21eowK8MwmDX11@ArchLinux","threadId":"64104","inReplyTo":null,"subject":"[PATCH 1/4] string-list: allow passing NULL for `get_entry_index`","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2025-09-07T16:40:26Z","receivedAt":"2025-09-07T16:40:28Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"Callers of `get_entry_index()` are required to pass a non-NULL\n`exact_match` parameter to receive information about whether an exact\nmatch is found. However, in some cases, callers only need the index\nposition.\n\nLet's allow callers to pass NULL for the `exact_match` parameter\nwhen they don't need this information, reducing unnecessary variable\ndeclarations in calling code.\n\nSigned-off-by: shejialuo \n---\n string-list.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/string-list.c b/string-list.c\nindex 343cf1ca90..bf358d1a5c 100644\n--- a/string-list.c\n+++ b/string-list.c\n@@ -29,12 +29,14 @@ static size_t get_entry_index(const struct string_list *list, const char *string\n \t\telse if (compare > 0)\n \t\t\tleft = middle + 1;\n \t\telse {\n-\t\t\t*exact_match = 1;\n+\t\t\tif (exact_match)\n+\t\t\t\t*exact_match = 1;\n \t\t\treturn middle;\n \t\t}\n \t}\n \n-\t*exact_match = 0;\n+\tif (exact_match)\n+\t\t*exact_match = 0;\n \treturn right;\n }\n \n-- \n2.51.0\n\n"},{"id":"525861","messageId":"xmqq5xdsyjoz.fsf@gitster.g","threadId":"64104","inReplyTo":"aL21eowK8MwmDX11@ArchLinux","subject":"Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-08T16:48:12Z","receivedAt":"2025-09-08T16:48:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"shejialuo writes:\n\n> Callers of `get_entry_index()` are required to pass a non-NULL\n> `exact_match` parameter to receive information about whether an exact\n> match is found. However, in some cases, callers only need the index\n> position.\n>\n> Let's allow callers to pass NULL for the `exact_match` parameter\n> when they don't need this information, reducing unnecessary variable\n> declarations in calling code.\n>\n> Signed-off-by: shejialuo \n> ---\n> string-list.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n\nI do not quite see the point of adding these conditional assignments\nto clutter the control flow. What benefit do these callers gain by\nnot having to have a throw-away int variable on the stack and\npassing its address to the call chain?\n\n> diff --git a/string-list.c b/string-list.c\n> index 343cf1ca90..bf358d1a5c 100644\n> --- a/string-list.c\n> +++ b/string-list.c\n> @@ -29,12 +29,14 @@ static size_t get_entry_index(const struct string_list *list, const char *string\n> \t\telse if (compare > 0)\n> \t\t\tleft = middle + 1;\n> \t\telse {\n> -\t\t\t*exact_match = 1;\n> +\t\t\tif (exact_match)\n> +\t\t\t\t*exact_match = 1;\n> \t\t\treturn middle;\n> \t\t}\n> \t}\n> \n> -\t*exact_match = 0;\n> +\tif (exact_match)\n> +\t\t*exact_match = 0;\n> \treturn right;\n> }\n"},{"id":"525999","messageId":"4D8F4C73-5447-4588-AA8A-7DC0646892B6@gmail.com","threadId":"64104","inReplyTo":"xmqq5xdsyjoz.fsf@gitster.g","subject":"Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`","fromName":"Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-09-10T00:09:52Z","receivedAt":"2025-09-10T00:10:04Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"\n> Le 8 sept. 2025 à 12:48, Junio C Hamano a écrit :\n> \n> shejialuo writes:\n> \n>> Callers of `get_entry_index()` are required to pass a non-NULL\n>> `exact_match` parameter to receive information about whether an exact\n>> match is found. However, in some cases, callers only need the index\n>> position.\n>> \n>> Let's allow callers to pass NULL for the `exact_match` parameter\n>> when they don't need this information, reducing unnecessary variable\n>> declarations in calling code.\n>> \n>> Signed-off-by: shejialuo \n>> ---\n>> string-list.c | 6 ++++--\n>> 1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> I do not quite see the point of adding these conditional assignments\n> to clutter the control flow. What benefit do these callers gain by\n> not having to have a throw-away int variable on the stack and\n> passing its address to the call chain?\n\nWouldn’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?"},{"id":"526000","messageId":"xmqqfrcvt7mf.fsf@gitster.g","threadId":"64104","inReplyTo":"4D8F4C73-5447-4588-AA8A-7DC0646892B6@gmail.com","subject":"Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-10T01:32:24Z","receivedAt":"2025-09-10T01:32:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Knoble writes:\n\n>> Le 8 sept. 2025 à 12:48, Junio C Hamano a écrit :\n>> \n>> shejialuo writes:\n>> \n>>> Callers of `get_entry_index()` are required to pass a non-NULL\n>>> `exact_match` parameter to receive information about whether an exact\n>>> match is found. However, in some cases, callers only need the index\n>>> position.\n>>> \n>>> Let's allow callers to pass NULL for the `exact_match` parameter\n>>> when they don't need this information, reducing unnecessary variable\n>>> declarations in calling code.\n>>> \n>>> Signed-off-by: shejialuo \n>>> ---\n>>> string-list.c | 6 ++++--\n>>> 1 file changed, 4 insertions(+), 2 deletions(-)\n>> \n>> I do not quite see the point of adding these conditional assignments\n>> to clutter the control flow. What benefit do these callers gain by\n>> not having to have a throw-away int variable on the stack and\n>> passing its address to the call chain?\n>\n> 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?\n\nYeah, but the point is in what situation does a caller *not* need to\nbe able to tell between \"we found an existing one and this is its\nindex\" vs \"there is no such thing, but here is where it *would* fit\nif it were placed in the array\"? I do not think of any, so I\ndoubted if it makes much sense to complicate the interface to make\nit optional.\n\n"},{"id":"526003","messageId":"DBE9993A-D6CC-4ABF-8D8B-18D54CF1B001@gmail.com","threadId":"64104","inReplyTo":"xmqqfrcvt7mf.fsf@gitster.g","subject":"Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`","fromName":"Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-09-10T02:04:20Z","receivedAt":"2025-09-10T02:04:32Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"\n> Le 9 sept. 2025 à 21:32, Junio C Hamano a écrit :\n> \n> Ben Knoble writes:\n> \n>>>> Le 8 sept. 2025 à 12:48, Junio C Hamano a écrit :\n>>> \n>>> shejialuo writes:\n>>> \n>>>> Callers of `get_entry_index()` are required to pass a non-NULL\n>>>> `exact_match` parameter to receive information about whether an exact\n>>>> match is found. However, in some cases, callers only need the index\n>>>> position.\n>>>> \n>>>> Let's allow callers to pass NULL for the `exact_match` parameter\n>>>> when they don't need this information, reducing unnecessary variable\n>>>> declarations in calling code.\n>>>> \n>>>> Signed-off-by: shejialuo \n>>>> ---\n>>>> string-list.c | 6 ++++--\n>>>> 1 file changed, 4 insertions(+), 2 deletions(-)\n>>> \n>>> I do not quite see the point of adding these conditional assignments\n>>> to clutter the control flow. What benefit do these callers gain by\n>>> not having to have a throw-away int variable on the stack and\n>>> passing its address to the call chain?\n>> \n>> 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?\n> \n> Yeah, but the point is in what situation does a caller *not* need to\n> be able to tell between \"we found an existing one and this is its\n> index\" vs \"there is no such thing, but here is where it *would* fit\n> if it were placed in the array\"? I do not think of any, so I\n> doubted if it makes much sense to complicate the interface to make\n> it optional.\n> \n\nGood question! Thanks for clarifying. "},{"id":"526355","messageId":"aMgBqnwKSKDC5g0I@ArchLinux","threadId":"64104","inReplyTo":"xmqq5xdsyjoz.fsf@gitster.g","subject":"Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2025-09-15T12:08:10Z","receivedAt":"2025-09-15T12:08:05Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Mon, Sep 08, 2025 at 09:48:12AM -0700, Junio C Hamano wrote:\n> shejialuo writes:\n> \n> > Callers of `get_entry_index()` are required to pass a non-NULL\n> > `exact_match` parameter to receive information about whether an exact\n> > match is found. However, in some cases, callers only need the index\n> > position.\n> >\n> > Let's allow callers to pass NULL for the `exact_match` parameter\n> > when they don't need this information, reducing unnecessary variable\n> > declarations in calling code.\n> >\n> > Signed-off-by: shejialuo \n> > ---\n> > string-list.c | 6 ++++--\n> > 1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> I do not quite see the point of adding these conditional assignments\n> to clutter the control flow. What benefit do these callers gain by\n> not having to have a throw-away int variable on the stack and\n> passing its address to the call chain?\n> \n\nI want to make the commit small. However, I have introduced confusion to\nthe reviewers. I would rebase the first commit and second commit into\none commit to improve this.\n\nThanks,\nJialuo\n"}]}