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

[Outreachy PATCH v5 0/2] do not use misdesigned strbuf_split*()

From
Olamide Caleb Bello <belkid98@gmail.com>
Date
Oct 22, 2025, 12:40 UTC
Message-ID
<cover.1761135129.git.belkid98@gmail.com>
In-Reply-To
<cover.1760997183.git.belkid98@gmail.com>

The patch series by Junio Hamano with link below, https://public-inbox.org/git/20250731225433.4028872-1-gitster@poddbox.com/, notices that the array of strbufs that calls to strbuf_split*() provides are merely used to store the strings gotten from the split and no edit are done on these resulting strings making the strbuf_split*() unideal for this usecase.

Commit d6fd08bd (sub-process: do not use strbuf_split*(), 2025-07-31) for example, in the series, observes that the subprocess_read_status() reads one packet line and tries to find "status=<foo>" by splitting the line into two strbufs which is an overkill to extract <foo>.

This series continues on this cleanup, by replacing instances of strbuf_split_max() with strchr() to get the required token around the delimiter where the token from the split is merely returned as char * and not strbufs and no edits are done on them. This makes the code cleaner, faster and more efficient.

Tests have also been performed on the commits on Github CI. The link is shown below
https://github.com/git/git/pull/2080
Changes in v5:
==============
 - Modify commit messages to provide proper context for commit reference
 - Correct reviewer's name and email address
 - correct code logic by assigning `fingerprint_stdout.buf` to `begin` in
   first call to strchr in patch 1
 - Modify logic in call to strchr() when no '\n' is found by passing '\0'
   to get the end of first line.
 - use die() in place of die_errno() in failed calls to strchr(), retaining
   die_errno() before the string is split
Olamide Caleb Bello (2):
  gpg-interface: do not use misdesigned strbuf_split*()
  gpg-interface: do not use misdesigned strbuf_split*()
 gpg-interface.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

Range diff versus v4 ====================

1:  2879d9be36 ! 1:  df8fbbd3a5 gpg-interface: do not use misdesigned strbuf_split*()
    @@ Commit message
     
         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
    +    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
    -    token.
    -    This removes the roundabout way of splitting the string into strbufs, just
    -    to return the token.
    +    token. This removes the roundabout way of splitting the string into
    +    strbufs just to return the token.
     
         Reported-by: Junio Hamano <gitster@pobox.com>
         Helped-by: Christian Couder <christian.couder@gmail.com>
    -    Helped-by: Junio Hamano <gitster@pobox.com>
    -    Helped-by: Krisoffer Haughsbakk
    +    Helped-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
         Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
     
      ## gpg-interface.c ##
    @@ gpg-interface.c: static char *get_ssh_key_fingerprint(const char *signing_key)
     -	if (!fingerprint[1])
     -		die_errno(_("failed to get the ssh fingerprint for key '%s'"),
     +	begin = fingerprint_stdout.buf;
    -+	delim = strchr(fingerprint_stdout.buf, ' ');
    ++	delim = strchr(begin, ' ');
     +	if (!delim)
    -+		die_errno(_("failed to get the ssh fingerprint for key %s"),
    ++		die(_("failed to get the ssh fingerprint for key %s"),
      			  signing_key);
     -
     -	fingerprint_ret = strbuf_detach(fingerprint[1], NULL);
    @@ gpg-interface.c: static char *get_ssh_key_fingerprint(const char *signing_key)
     +	begin = delim + 1;
     +	delim = strchr(begin, ' ');
     +	if (!delim)
    -+	    die_errno(_("failed to get the ssh fingerprint for key %s"),
    ++	    die(_("failed to get the ssh fingerprint for key %s"),
     +			  signing_key);
     +	fingerprint_ret = xmemdupz(begin, delim - begin);
      	strbuf_release(&fingerprint_stdout);
2:  a830de15ec ! 2:  5df667227b gpg-interface: do not use misdesigned strbuf_split*() [Part 2]
    @@ Metadata
     Author: Olamide Caleb Bello <belkid98@gmail.com>
     
      ## Commit message ##
    -    gpg-interface: do not use misdesigned strbuf_split*() [Part 2]
    +    gpg-interface: do not use misdesigned strbuf_split*()
     
         In get_default_ssh_signing_key(), the default ssh signing key is
         retrieved in `key_stdout` buf, which is then split using
    @@ Commit message
     
         Reported-by: Junio Hamano <gitster@pobox.com>
         Helped-by: Christian Couder <christian.couder@gmail.com>
    -    Helped-by: Junio Hamano <gitster@pobox.com>
    -    Helped-by: Krisoffer Haughsbakk
    +    Helped-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
         Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
     
      ## gpg-interface.c ##
    @@ 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;
    ++	char *begin, *new_line, *first_line, *end;
      
      	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');
    -+		first_line = xmemdupz(begin, new_line - begin);
    ++		end = new_line ? new_line : strchr(begin, '\0');
    ++		first_line = xmemdupz(begin, end - 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: Bello OlamideNext: Olamide Caleb Bello
Message 14 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.