Re: [Outreachy PATCH v3 1/2] gpg-interface: replace strbuf_split*() with string_list_split*()

[Junio C Hamano <gitster@pobox.com>]

Bello Olamide <belkid98@gmail.com> writes:

>> > -     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?

>     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
;-)