Subject: [Outreachy PATCH v3 0/2] gpg-interface.c: use string_list_split*() instead of strbuf_split*()

From: Olamide Caleb Bello <[email-masked]>
Date: Sun, 19 Oct 2025 12:07:41 +0000

The patch series by Junio Hamano with link below,
https://public-inbox.org/git/[email-masked]/,
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, with the string_list_split*() being a more suitable
option.

Commit 2efe707054 (wt-status: avoid strbuf_split*(), 2025-07-31) for example,
in the series, notes that abbrev_oid_in_line() takes one line of rebase
todo list and splits tokens out of this line using strbuf_split_max().
However, no simultanous edits that take advantage of the strbuf API take
place but the tokens are merely used as pieces of strings.

This series continues on this, by replacing instances of
strbuf_split_max() with string_list_split_in_place() where the string from
the split is merely returned as char * and no edits are done on them.

Tests have also been performed on the commits on Github CI. The link is shown below
https://github.com/git/git/pull/2074

Changes in v3:
==============
 - Use string_list_split_in_place() instead of string_list_split() used in v2
 - Change the test for the return value of string_list_split_in_place()
   in get_ssh_key_fingerprint() patch 1 to at least 2 tokens in the split
   list to allow a successful test where the key owner's identity is missing.
 - Change the test for the return value of string_list_split_in_place() in
   get_default_ssh_signing_key() in patch 2 to at least 1 token to allow for
   a successful test.
 - Modify commit messages to indicate the switch to string_list_split_in_place()
   from string_list_split() used in v2
 - Add explanation in commit message about the test for the return value of
   string_list_split_in_place() in both patches
 - Add Helped-by and Reported-by tags

Olamide Caleb Bello (2):
  gpg-interface: replace strbuf_split*() with string_list_split*()
  gpg-interface: use string_list_split*() instead of strbuf_split*()

 gpg-interface.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Range-diff versus v2:

1:  818ca6b104 ! 1:  7da4fded53 gpg-interface: replace strbuf_split_max() with string_list_split()
    @@ Metadata
     Author: Olamide Caleb Bello <[email-masked]>

      ## Commit message ##
    -    gpg-interface: replace strbuf_split_max() with string_list_split()
    +    gpg-interface: replace strbuf_split*() with string_list_split*()

         In get_ssh_finger_print(), the output of the `ssh-keygen` command is
         put into `fingerprint_stdout
    @@ Commit message
         The string in fingerprint_stdout is then split into 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.
    +    returned as a char * and not a strbuf, hence they do not need to be
    +    strbufs.

    -    Use string_list_split() instead for simplicity.
    +    Use string_list_split_in_place() instead for simplicity.

         Note that strbuf_split_max() uses 3 to specify the number of tokens to
    -    extract from the string, while string_list_split() uses 2 because it specifies
    -    the number of times the split will be done on the string, so 2 gives 3 tokens
    -    as it is in the original instance.
    +    extract from the string, while string_list_split_in_place() uses 2
    +    because it specifies the number of times the split will be done on
    +    the string, so 2 gives 3 tokens as it is in the original instance.
    +
    +    string_list_split_in_place() returns the number of substrings added to
    +    the `split.items` so for a successful split of the string in
    +    fingerprint_stdout, at least two items should be added to split.items
    +    so we can always be certain that the substring at index 1 is the ssh
    +    fingerprint even if the key owner's identity part is missing from the
    +    string in fingerprint_stdout.

         Signed-off-by: Olamide Caleb Bello <[email-masked]>
    +    Reported-by: Junio Hamano <[email-masked]>
    +    Helped-by: Christian Couder <[email-masked]>
    +    Helped-by: Junio Hamano <[email-masked]>

      ## gpg-interface.c ##
     @@
    @@ gpg-interface.c: static char *get_ssh_key_fingerprint(const char *signing_key)
      	int ret = -1;
      	struct strbuf fingerprint_stdout = STRBUF_INIT;
     -	struct strbuf **fingerprint;
    -+	struct string_list split = STRING_LIST_INIT_DUP;
    ++	struct string_list split = STRING_LIST_INIT_NODUP;
      	char *fingerprint_ret;
      	const char *literal_key = NULL;

    @@ gpg-interface.c: static char *get_ssh_key_fingerprint(const char *signing_key)

     -	fingerprint = strbuf_split_max(&fingerprint_stdout, ' ', 3);
     -	if (!fingerprint[1])
    -+	if (string_list_split(&split, fingerprint_stdout.buf, " ", 2) != 3)
    ++	if (string_list_split_in_place(&split, fingerprint_stdout.buf, " ", 2) <= 1)
      		die_errno(_("failed to get the ssh fingerprint for key '%s'"),
      			  signing_key);

2:  024a44a242 ! 2:  9a6eb6ff8b gpg-interface: use string_list_split() instead of strbuf_split_max()
    @@ Metadata
     Author: Olamide Caleb Bello <[email-masked]>

      ## Commit message ##
    -    gpg-interface: use string_list_split() instead of strbuf_split_max()
    +    gpg-interface: use string_list_split*() instead of strbuf_split*()

         In get_default_ssh_signing_key(), the default ssh signing key is
         retrieved in `key_stdout`, which is then split using
    @@ Commit message
         This makes the function lack the use of strbuf API as no edits are
         performed on the split tokens.

    -    Replace strbuf_split_max() with string_list_split() for simplicity.
    +    Replace strbuf_split_max() with string_list_split_in_place() for
    +    simplicity

         Note that strbuf_split_max() uses `2` to indicate the number of tokens
    -    to extract from the string, while string_list_split() uses `1` to specify
    -    the number of times the split will be done on the string, so 1 gives 2
    -    tokens as it is in the original instance.
    +    to extract from the string, while string_list_split_in_place() uses `1`
    +    to specify the number of times the split will be done on the string,
    +    so 1 gives 2 tokens as it is in the original instance.
    +
    +    string_list_split_in_place() returns the number of substrings added to the
    +    list keys.items, so we check that at least one substring is added to the
    +    list since we just want to return the first substring.

         Signed-off-by: Olamide Caleb Bello <[email-masked]>
    +    Reported-by: Junio Hamano <[email-masked]>
    +    Helped-by: Christian Couder <[email-masked]>

      ## gpg-interface.c ##
     @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)
    @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)
      	int ret = -1;
      	struct strbuf key_stdout = STRBUF_INIT, key_stderr = STRBUF_INIT;
     -	struct strbuf **keys;
    -+	struct string_list keys = STRING_LIST_INIT_DUP;
    ++	struct string_list keys = STRING_LIST_INIT_NODUP;
      	char *key_command = NULL;
      	const char **argv;
      	int n;
    @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)
      	if (!ret) {
     -		keys = strbuf_split_max(&key_stdout, '\n', 2);
     -		if (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {
    -+		if (string_list_split(&keys, key_stdout.buf, "\n", 1) == 2 &&
    ++		if (string_list_split_in_place(&keys, key_stdout.buf, "\n", 1) > 0 &&
     +			is_literal_ssh_key(keys.items[0].string, &literal_key)) {
      			/*
      			 * We only use `is_literal_ssh_key` here to check validity

-- 
2.51.0.463.g79cf913ea9