git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Olamide Caleb BelloNext: Christian Couder
Message 25 of 26 in “do not use strbuf_split*()”
  1. 0/2 do not use strbuf_split*()Olamide Caleb Bello, Oct 20, 2025
  2. 1/2 gpg-interface: do not use misdesigned strbuf_split*()Olamide Caleb Bello, Oct 20, 2025
  3. Christian CouderOct 21, 2025
  4. Christian CouderOct 21, 2025
  5. Bello OlamideOct 21, 2025
  6. Junio C HamanoOct 21, 2025
  7. 2/2 gpg-interface: do not use misdesigned strbuf_split*() [Part 2]Olamide Caleb Bello, Oct 20, 2025
  8. Christian CouderOct 21, 2025
  9. Bello OlamideOct 21, 2025
  10. Christian CouderOct 21, 2025
  11. Bello OlamideOct 21, 2025
  12. Junio C HamanoOct 21, 2025
  13. Bello OlamideOct 22, 2025
  14. 0/2 do not use misdesigned strbuf_split*()Olamide Caleb Bello, Oct 22, 2025
  15. 1/2 gpg-interface: do not use misdesigned strbuf_split*()Olamide Caleb Bello, Oct 22, 2025
  16. Christian CouderOct 22, 2025
  17. Bello OlamideOct 23, 2025
  18. 2/2 gpg-interface: do not use misdesigned strbuf_split*()Olamide Caleb Bello, Oct 22, 2025
  19. Christian CouderOct 22, 2025
  20. Junio C HamanoOct 22, 2025
  21. Bello OlamideOct 23, 2025
  22. 0/2 do not use misdesigned strbuf_split*()Olamide Caleb Bello, Oct 23, 2025
  23. 1/2 gpg-interface: do not use misdesigned strbuf_split*()Olamide Caleb Bello, Oct 23, 2025
  24. 2/2 gpg-interface: do not use misdesigned strbuf_split*()Olamide Caleb Bello, Oct 23, 2025
  25. Junio C HamanoOct 23, 2025
  26. Christian CouderOct 24, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.