Re: [Outreachy PATCH v6 0/2] do not use misdesigned strbuf_split*()

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

Olamide Caleb Bello <belkid98@gmail.com> writes:

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

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