From: Bello Olamide <belkid98@gmail.com>
Date: Thu, 23 Oct 2025 08:17:31 GMT
Subject: Re: [Outreachy PATCH v5 2/2] gpg-interface: do not use misdesigned strbuf_split*()
Message-ID: <CAD=f0L9qsVOo5=2XfKxd-UvzzzJ=PEE0-kW=wyOxSVvTYt7Vyw@mail.gmail.com>
In-Reply-To: <CAP8UFD3OTMi6uxv+z4rTqJ4wVpmezSG2Yj8tZMpgptWaWU343w@mail.gmail.com>

On Wed, 22 Oct 2025 at 15:04, Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Wed, Oct 22, 2025 at 2:40 PM Olamide Caleb Bello <belkid98@gmail.com> wrote:
>
> [...]
>
> > Simplify the process of retrieving and returning the desired line by
> > using strchr() to isolate the line and xmemdupz() to return a copy of the
> > line.
> > This removes the roundabout way of splitting the string into strbufs, just
> > to return the line.
>
> Nit: here also I think it should be clear that these last two
> sentences are in the same paragraph.

Okay

>
> [...]
>
> > @@ -887,19 +887,22 @@ static char *get_default_ssh_signing_key(void)
> >                            &key_stderr, 0);
> >
> >         if (!ret) {
> > -               keys = strbuf_split_max(&key_stdout, '\n', 2);
> > -               if (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {
> > +               begin = key_stdout.buf;
> > +               new_line = strchr(begin, '\n');
> > +               end = new_line ? new_line : strchr(begin, '\0');
> > +               first_line = xmemdupz(begin, end - begin);
>
> That works but I wonder if something like the following is not a bit better:
>
>                if (new_line)
>                        first_line = xmemdupz(begin, new_line - begin);
>                else
>                        first_line = xstrdup(begin);

Ah yes.
It is much better.
Thank you very much for your guide.
I have already learnt a lot in this series.

Bello