Olamide Caleb BelloThe 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 in those cases.
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 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/2076
Changes in v4: ============== - Use strchr() to extract the required token over string_list_split_in_place used in v3. - Modify commit messages to indicate the switch to strchr() from string_list_split_in_place() used in v2 and reason for prefering strchr()
Olamide Caleb Bello (2): gpg-interface: do not use misdesigned strbuf_split*() gpg-interface: do not use misdesigned strbuf_split*() [Part 2]
gpg-interface.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-)
Range diff versus v3
====================
1: 7da4fded53 < -: ---------- gpg-interface: replace strbuf_split*() with string_list_split*()
-: ---------- > 1: 2879d9be36 gpg-interface: do not use misdesigned strbuf_split*()
2: 9a6eb6ff8b ! 2: a830de15ec gpg-interface: use string_list_split*() instead of strbuf_split*()
@@ Metadata
Author: Olamide Caleb Bello <belkid98@gmail.com> ## Commit message ##
- gpg-interface: use string_list_split*() instead of strbuf_split*()
+ gpg-interface: do not use misdesigned strbuf_split*() [Part 2] In get_default_ssh_signing_key(), the default ssh signing key is
- retrieved in `key_stdout`, which is then split using
- strbuf_split_max() into two tokens
-
- The string in `key_stdout` is then split using strbuf_split_max() into
- two tokens at a new line and the first token is returned as a `char *`
- and not a strbuf.
+ retrieved in `key_stdout` buf, which is then split using
+ strbuf_split_max() into up to two strbufs at a new line and the first
+ strbuf is returned as a `char *`and not a strbuf.
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_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_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.
+ 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. - Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
- Reported-by: Junio Hamano <gister@pobox.com>
+ 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
+ Signed-off-by: Olamide Caleb Bello <belkid98@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_NODUP;
char *key_command = NULL;
const char **argv;
int n;
+ char *default_key = NULL;
+ const char *literal_key = NULL;
++ 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)
&key_stderr, 0); 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_in_place(&keys, key_stdout.buf, "\n", 1) > 0 &&
-+ is_literal_ssh_key(keys.items[0].string, &literal_key)) {
++ begin = key_stdout.buf;
++ new_line = strchr(begin, '\n');
++ first_line = xmemdupz(begin, new_line - begin);
++ if (is_literal_ssh_key(first_line, &literal_key)) {
/*
* We only use `is_literal_ssh_key` here to check validity
* The prefix will be stripped when the key is used.
*/
- default_key = strbuf_detach(keys[0], NULL);
-+ default_key = xstrdup(keys.items[0].string);
++ default_key = first_line;
} else {
++ free(first_line);
warning(_("gpg.ssh.defaultKeyCommand succeeded but returned no keys: %s %s"),
key_stderr.buf, key_stdout.buf);
} - strbuf_list_free(keys);
-+ string_list_clear(&keys, 0);
} else {
warning(_("gpg.ssh.defaultKeyCommand failed: %s %s"),
key_stderr.buf, key_stdout.buf);-- 2.51.0.463.g79cf913ea9
