From: Junio C Hamano Date: Thu, 23 Oct 2025 16:27:34 GMT Subject: Re: [Outreachy PATCH v6 0/2] do not use misdesigned strbuf_split*() Message-ID: In-Reply-To: Olamide Caleb Bello 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 > Helped-by: Christian Couder > @@ 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