Re: [Outreachy PATCH v6 0/2] do not use misdesigned strbuf_split*()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 23, 2025, 16:27 UTC
- Message-ID
- <xmqqecqtwpl5.fsf@gitster.g>
- In-Reply-To
- <cover.1761217100.git.belkid98@gmail.com>
Olamide Caleb Bello <belkid98@gmail.com> writes:
Show 5 quoted lines
> Changes in v6 > ============= > - Modify commit messages to have proper structure > - Changed logic in get_default_ssh_signing_key() to use xmemdupz() if > key has '\n' and xstrdup() if not.
This round looks good to me. Christian, should we declare victory and mark it for 'next' now?
Thanks.
Show 68 quoted lines
>
> Olamide Caleb Bello (2):
> gpg-interface: do not use misdesigned strbuf_split*()
> gpg-interface: do not use misdesigned strbuf_split*()
>
> gpg-interface.c | 34 +++++++++++++++++++++-------------
> 1 file changed, 21 insertions(+), 13 deletions(-)
>
> Range diff versus v5
> ====================
>
> 1: df8fbbd3a5 ! 1: 92fc78c203 gpg-interface: do not use misdesigned strbuf_split*()
> @@ Commit message
> gpg-interface: do not use misdesigned strbuf_split*()
>
> In get_ssh_finger_print(), the output of the `ssh-keygen` command is
> - put into `fingerprint_stdout` strbuf.
> - The string in `fingerprint_stdout` is then split into up to 3 strbufs
> - using strbuf_split_max(). However they are not modified after the split
> - thereby not making use of the strbuf API as the fingerprint token is
> - merely returned as a char * and not a strbuf. Hence they do not need to be
> - strbufs.
> + put into `fingerprint_stdout` strbuf. The string in `fingerprint_stdout`
> + is then split into up to 3 strbufs using strbuf_split_max(). However they
> + are not modified after the split thereby not making use of the strbuf API
> + as the fingerprint token is merely returned as a char * and not a strbuf.
> + Hence they do not need to be strbufs.
>
> Simplify the process of retrieving and returning the desired token by
> using strchr() to isolate the token and xmemdupz() to return a copy of the
> 2: 5df667227b ! 2: e52855242c gpg-interface: do not use misdesigned strbuf_split*()
> @@ Commit message
>
> 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.
> + line. This removes the roundabout way of splitting the string into
> + strbufs, just to return the line.
>
> Reported-by: Junio Hamano <gitster@pobox.com>
> Helped-by: Christian Couder <christian.couder@gmail.com>
> @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)
> int n;
> char *default_key = NULL;
> const char *literal_key = NULL;
> -+ char *begin, *new_line, *first_line, *end;
> ++ char *begin, *new_line, *first_line;
>
> if (!ssh_default_key_command)
> die(_("either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured"));
> @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)
> - 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);
> ++ if (new_line)
> ++ first_line = xmemdupz(begin, new_line - begin);
> ++ else
> ++ first_line = xstrdup(begin);
> + if (is_literal_ssh_key(first_line, &literal_key)) {
> /*
> * We only use `is_literal_ssh_key` here to check validity
>
> --
> 2.51.0.463.g79cf913ea9