git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Ben Knoble <ben.knoble@gmail.com>
Cc: shejialuo <shejialuo@gmail.com>,  git@vger.kernel.org
Subject: Re: [PATCH 1/4] string-list: allow passing NULL for `get_entry_index`
Date: Tue, 09 Sep 2025 18:32:24 -0700	[thread overview]
Message-ID: <xmqqfrcvt7mf.fsf@gitster.g> (raw)
In-Reply-To: <4D8F4C73-5447-4588-AA8A-7DC0646892B6@gmail.com> (Ben Knoble's message of "Tue, 9 Sep 2025 20:09:52 -0400")

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.


  reply	other threads:[~2025-09-10  1:32 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqfrcvt7mf.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=ben.knoble@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=shejialuo@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).