From: Olamide Caleb Bello <belkid98@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, usmanakinyemi202@gmail.com,
christian.couder@gmail.com,
Olamide Caleb Bello <belkid98@gmail.com>
Subject: [Outreachy PATCH v3 0/2] gpg-interface.c: use string_list_split*() instead of strbuf_split*()
Date: Sun, 19 Oct 2025 12:07:41 +0000 [thread overview]
Message-ID: <cover.1760869186.git.belkid98@gmail.com> (raw)
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, 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 <belkid98@gmail.com>
## 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 <belkid98@gmail.com>
+ Reported-by: Junio Hamano <gitster@pobox.com>
+ Helped-by: Christian Couder <christian.couder@gmail.com>
+ Helped-by: Junio Hamano <gitster@pobox.com>
## 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 <belkid98@gmail.com>
## 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 <belkid98@gmail.com>
+ Reported-by: Junio Hamano <gister@pobox.com>
+ Helped-by: Christian Couder <christian.couder@gmail.com>
## 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
next reply other threads:[~2025-10-19 12:09 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-19 12:07 Olamide Caleb Bello [this message]
2025-10-19 12:07 ` [Outreachy PATCH v3 1/2] gpg-interface: replace strbuf_split*() with string_list_split*() Olamide Caleb Bello
2025-10-19 15:52 ` Junio C Hamano
2025-10-20 6:32 ` Bello Olamide
2025-10-20 15:09 ` Junio C Hamano
2025-10-20 16:31 ` Junio C Hamano
2025-10-20 18:15 ` Bello Olamide
2025-10-20 18:12 ` Bello Olamide
2025-10-20 16:45 ` Kristoffer Haugsbakk
2025-10-20 18:25 ` Bello Olamide
2025-10-20 18:37 ` Kristoffer Haugsbakk
2025-10-20 18:50 ` Bello Olamide
2025-10-19 12:07 ` [Outreachy PATCH v3 2/2] gpg-interface: use string_list_split*() instead of strbuf_split*() Olamide Caleb Bello
2025-10-19 16:00 ` Junio C Hamano
2025-10-20 8:15 ` Bello Olamide
2025-10-20 15:18 ` Junio C Hamano
2025-10-20 19:02 ` Bello Olamide
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1760869186.git.belkid98@gmail.com \
--to=belkid98@gmail.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=usmanakinyemi202@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).