Re: [Outreachy PATCH v3 1/2] gpg-interface: replace strbuf_split*() with string_list_split*()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 20, 2025, 15:09 UTC
- Message-ID
- <xmqq5xc9390j.fsf@gitster.g>
- In-Reply-To
- <CAD=f0L8Zc=rRwCo0QHWnf8OCXidubJrQYpeQg=gLXV=npO-T1g@mail.gmail.com>
Bello Olamide <belkid98@gmail.com> writes:
Show 20 quoted lines
>> > - fingerprint_ret = strbuf_detach(fingerprint[1], NULL);
>> > - strbuf_list_free(fingerprint);
>> > + fingerprint_ret = xstrdup(split.items[1].string);
>> > + string_list_clear(&split, 0);
>>
>> OK. This is a straight-forward rewrite that is fairly faithful to
>> the original.
>>
>> But I wonder why the original was written in such a convoluted way
>> to just extract the first part of a string that is space delimited
>> tokens. It is obviously not your fault that the original is written
>> that way, bit I would have expected it to be done more like this:
>>
>> char *begin = fingerprint_stdout.buf;
>> char *delim = strchr(begin, ' ');
>> if (!delim)
>> die_errno("Barf!");
>> fingerprint_ret = xmemdupz(begin, end - begin);
>>
>> Am I missing something?What I was missing was that we use fingerprint[1], not fingerprint[0]. So we need to do the strchr() twice, i.e.
char *begin = fingerprint_stdout.buf;
char *delim = strchr(begin, ' ');
if (!delim)
die_errno("Barf!");
begin = delim + 1
delim = strchr(begin, ' ');
if (!delim)
die_errno("Barf!");
fingerprint_ret = xmemdupz(begin, end - begin);> Okay something like this which just finds the desired token and > returns a copy?
Show 11 quoted lines
> char *begin = fingerprint_stdout.buf;
> char *end = begin + fingerprint_stdout.len;
> char *space, *start, *endtok;
>
> space = memchr(begin, ' ', end-begin);
> if (!space)
> die_errno(_("failed to get the ssh fingerprint for key '%s'"),
> signing_key);
> start = space + 1;
> while (start < end && (*start = ' ' || *start == '\t'))
> start++;The original does not seem to care and uses the whole fingerprint[1].buf; do we really care about tabs? The same for looking at CR or LF.
Even if we cared, we shouldn't have to open code strcspn() like this ;-)