{"thread":{"id":"64360","subject":"[Outreachy PATCH v4 0/2] do not use strbuf_split*()","startedAt":"2025-10-20T22:56:40Z","lastAt":"2025-10-24T13:25:22Z","messageCount":26,"participants":["Olamide Caleb Bello","Christian Couder","Bello Olamide","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"529210","messageId":"cover.1760997183.git.belkid98@gmail.com","threadId":"64360","inReplyTo":null,"subject":"[Outreachy PATCH v4 0/2] do not use strbuf_split*()","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-20T22:55:19Z","receivedAt":"2025-10-20T22:56:40Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"The patch series by Junio Hamano with link below,\nhttps://public-inbox.org/git/20250731225433.4028872-1-gitster@poddbox.com/,\nnotices that the array of strbufs that calls to strbuf_split*() provides\nare merely used to store the strings gotten from the split and no edit are\ndone on these resulting strings making the strbuf_split*() unideal\nfor this usecase, with the string_list_split*() being a more suitable\noption in those cases.\n\nCommit 2efe707054 (wt-status: avoid strbuf_split*(), 2025-07-31) for example,\nin the series, notes that abbrev_oid_in_line() takes one line of rebase\ntodo list and splits tokens out of this line using strbuf_split_max().\nHowever, no simultanous edits that take advantage of the strbuf API take\nplace but the tokens are merely used as pieces of strings.\n\nThis series continues on this cleanup, by replacing instances of\nstrbuf_split_max() with strchr() to get the required token around the\ndelimiter where the token from the split is merely returned as char *\nand not strbufs and no edits are done on them.\nThis makes the code cleaner, faster and more efficient.\n\nTests have also been performed on the commits on Github CI. The link is shown below\n\nhttps://github.com/git/git/pull/2076\n\nChanges in v4:\n==============\n - Use strchr() to extract the required token over string_list_split_in_place\n used in v3.\n - Modify commit messages to indicate the switch to strchr() from\n string_list_split_in_place() used in v2 and reason for prefering strchr()\n\nOlamide Caleb Bello (2):\n gpg-interface: do not use misdesigned strbuf_split*()\n gpg-interface: do not use misdesigned strbuf_split*() [Part 2]\n\n gpg-interface.c | 31 ++++++++++++++++++-------------\n 1 file changed, 18 insertions(+), 13 deletions(-)\n\nRange diff versus v3\n====================\n1: 7da4fded53 < -: ---------- gpg-interface: replace strbuf_split*() with string_list_split*()\n-: ---------- > 1: 2879d9be36 gpg-interface: do not use misdesigned strbuf_split*()\n2: 9a6eb6ff8b ! 2: a830de15ec gpg-interface: use string_list_split*() instead of strbuf_split*()\n @@ Metadata\n Author: Olamide Caleb Bello \n\n ## Commit message ##\n - gpg-interface: use string_list_split*() instead of strbuf_split*()\n + gpg-interface: do not use misdesigned strbuf_split*() [Part 2]\n\n In get_default_ssh_signing_key(), the default ssh signing key is\n - retrieved in `key_stdout`, which is then split using\n - strbuf_split_max() into two tokens\n -\n - The string in `key_stdout` is then split using strbuf_split_max() into\n - two tokens at a new line and the first token is returned as a `char *`\n - and not a strbuf.\n + retrieved in `key_stdout` buf, which is then split using\n + strbuf_split_max() into up to two strbufs at a new line and the first\n + strbuf is returned as a `char *`and not a strbuf.\n This makes the function lack the use of strbuf API as no edits are\n performed on the split tokens.\n\n - Replace strbuf_split_max() with string_list_split_in_place() for\n - simplicity\n -\n - Note that strbuf_split_max() uses `2` to indicate the number of tokens\n - to extract from the string, while string_list_split_in_place() uses `1`\n - to specify the number of times the split will be done on the string,\n - so 1 gives 2 tokens as it is in the original instance.\n -\n - string_list_split_in_place() returns the number of substrings added to the\n - list keys.items, so we check that at least one substring is added to the\n - list since we just want to return the first substring.\n + Simplify the process of retrieving and returning the desired line by\n + using strchr() to isolate the line and xmemdupz() to return a copy of the\n + line.\n + This removes the roundabout way of splitting the string into strbufs, just\n + to return the line.\n\n - Signed-off-by: Olamide Caleb Bello \n - Reported-by: Junio Hamano \n + Reported-by: Junio Hamano \n Helped-by: Christian Couder \n + Helped-by: Junio Hamano \n + Helped-by: Krisoffer Haughsbakk\n + Signed-off-by: Olamide Caleb Bello \n\n ## gpg-interface.c ##\n @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)\n @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)\n \tint ret = -1;\n \tstruct strbuf key_stdout = STRBUF_INIT, key_stderr = STRBUF_INIT;\n -\tstruct strbuf **keys;\n -+\tstruct string_list keys = STRING_LIST_INIT_NODUP;\n \tchar *key_command = NULL;\n \tconst char **argv;\n \tint n;\n + \tchar *default_key = NULL;\n + \tconst char *literal_key = NULL;\n ++\tchar *begin, *new_line, *first_line;\n +\n + \tif (!ssh_default_key_command)\n + \t\tdie(_(\"either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured\"));\n @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)\n \t\t\t &key_stderr, 0);\n\n \tif (!ret) {\n -\t\tkeys = strbuf_split_max(&key_stdout, '\\n', 2);\n -\t\tif (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n -+\t\tif (string_list_split_in_place(&keys, key_stdout.buf, \"\\n\", 1) > 0 &&\n -+\t\t\tis_literal_ssh_key(keys.items[0].string, &literal_key)) {\n ++\t\tbegin = key_stdout.buf;\n ++\t\tnew_line = strchr(begin, '\\n');\n ++\t\tfirst_line = xmemdupz(begin, new_line - begin);\n ++\t\tif (is_literal_ssh_key(first_line, &literal_key)) {\n \t\t\t/*\n \t\t\t * We only use `is_literal_ssh_key` here to check validity\n \t\t\t * The prefix will be stripped when the key is used.\n \t\t\t */\n -\t\t\tdefault_key = strbuf_detach(keys[0], NULL);\n -+\t\t\tdefault_key = xstrdup(keys.items[0].string);\n ++\t\t\tdefault_key = first_line;\n \t\t} else {\n ++\t\t\tfree(first_line);\n \t\t\twarning(_(\"gpg.ssh.defaultKeyCommand succeeded but returned no keys: %s %s\"),\n \t\t\t\tkey_stderr.buf, key_stdout.buf);\n \t\t}\n\n -\t\tstrbuf_list_free(keys);\n -+\t\tstring_list_clear(&keys, 0);\n \t} else {\n \t\twarning(_(\"gpg.ssh.defaultKeyCommand failed: %s %s\"),\n \t\t\tkey_stderr.buf, key_stdout.buf);\n\n\n-- \n2.51.0.463.g79cf913ea9\n\n"},{"id":"529211","messageId":"2879d9be3659a9c1ea554fff7814507caae24b65.1760997183.git.belkid98@gmail.com","threadId":"64360","inReplyTo":"cover.1760997183.git.belkid98@gmail.com","subject":"[Outreachy PATCH v4 1/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-20T22:55:20Z","receivedAt":"2025-10-20T22:56:44Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"In get_ssh_finger_print(), the output of the `ssh-keygen` command is\nput into `fingerprint_stdout` strbuf.\n\nThe string in fingerprint_stdout is then split into up to 3 strbufs using\nstrbuf_split_max(), however they are not modified after the split thereby\nnot making use of the strbuf API as the fingerprint token is merely\nreturned as a char * and not a strbuf, hence they do not need to be\nstrbufs.\n\nSimplify the process of retrieving and returning the desired token by\nusing strchr() to isolate the token and xmemdupz() to return a copy of the\ntoken.\nThis removes the roundabout way of splitting the string into strbufs, just\nto return the token.\n\nReported-by: Junio Hamano \nHelped-by: Christian Couder \nHelped-by: Junio Hamano \nHelped-by: Krisoffer Haughsbakk\nSigned-off-by: Olamide Caleb Bello \n---\n gpg-interface.c | 19 +++++++++++--------\n 1 file changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 2f4f0e32cb..1d793a56d2 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -821,8 +821,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n \tstruct child_process ssh_keygen = CHILD_PROCESS_INIT;\n \tint ret = -1;\n \tstruct strbuf fingerprint_stdout = STRBUF_INIT;\n-\tstruct strbuf **fingerprint;\n-\tchar *fingerprint_ret;\n+\tchar *fingerprint_ret, *begin, *delim;\n \tconst char *literal_key = NULL;\n \n \t/*\n@@ -845,13 +844,17 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n \t\tdie_errno(_(\"failed to get the ssh fingerprint for key '%s'\"),\n \t\t\t signing_key);\n \n-\tfingerprint = strbuf_split_max(&fingerprint_stdout, ' ', 3);\n-\tif (!fingerprint[1])\n-\t\tdie_errno(_(\"failed to get the ssh fingerprint for key '%s'\"),\n+\tbegin = fingerprint_stdout.buf;\n+\tdelim = strchr(fingerprint_stdout.buf, ' ');\n+\tif (!delim)\n+\t\tdie_errno(_(\"failed to get the ssh fingerprint for key %s\"),\n \t\t\t signing_key);\n-\n-\tfingerprint_ret = strbuf_detach(fingerprint[1], NULL);\n-\tstrbuf_list_free(fingerprint);\n+\tbegin = delim + 1;\n+\tdelim = strchr(begin, ' ');\n+\tif (!delim)\n+\t die_errno(_(\"failed to get the ssh fingerprint for key %s\"),\n+\t\t\t signing_key);\n+\tfingerprint_ret = xmemdupz(begin, delim - begin);\n \tstrbuf_release(&fingerprint_stdout);\n \treturn fingerprint_ret;\n }\n-- \n2.51.0.463.g79cf913ea9\n\n"},{"id":"529212","messageId":"a830de15ecdb5e5f45625927cb69b2be552bda42.1760997183.git.belkid98@gmail.com","threadId":"64360","inReplyTo":"cover.1760997183.git.belkid98@gmail.com","subject":"[Outreachy PATCH v4 2/2] gpg-interface: do not use misdesigned strbuf_split*() [Part 2]","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-20T22:55:21Z","receivedAt":"2025-10-20T22:57:06Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"In get_default_ssh_signing_key(), the default ssh signing key is\nretrieved in `key_stdout` buf, which is then split using\nstrbuf_split_max() into up to two strbufs at a new line and the first\nstrbuf is returned as a `char *`and not a strbuf.\nThis makes the function lack the use of strbuf API as no edits are\nperformed on the split tokens.\n\nSimplify the process of retrieving and returning the desired line by\nusing strchr() to isolate the line and xmemdupz() to return a copy of the\nline.\nThis removes the roundabout way of splitting the string into strbufs, just\nto return the line.\n\nReported-by: Junio Hamano \nHelped-by: Christian Couder \nHelped-by: Junio Hamano \nHelped-by: Krisoffer Haughsbakk\nSigned-off-by: Olamide Caleb Bello \n---\n gpg-interface.c | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 1d793a56d2..420e3a6646 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -865,12 +865,12 @@ static char *get_default_ssh_signing_key(void)\n \tstruct child_process ssh_default_key = CHILD_PROCESS_INIT;\n \tint ret = -1;\n \tstruct strbuf key_stdout = STRBUF_INIT, key_stderr = STRBUF_INIT;\n-\tstruct strbuf **keys;\n \tchar *key_command = NULL;\n \tconst char **argv;\n \tint n;\n \tchar *default_key = NULL;\n \tconst char *literal_key = NULL;\n+\tchar *begin, *new_line, *first_line;\n \n \tif (!ssh_default_key_command)\n \t\tdie(_(\"either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured\"));\n@@ -887,19 +887,21 @@ static char *get_default_ssh_signing_key(void)\n \t\t\t &key_stderr, 0);\n \n \tif (!ret) {\n-\t\tkeys = strbuf_split_max(&key_stdout, '\\n', 2);\n-\t\tif (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n+\t\tbegin = key_stdout.buf;\n+\t\tnew_line = strchr(begin, '\\n');\n+\t\tfirst_line = xmemdupz(begin, new_line - begin);\n+\t\tif (is_literal_ssh_key(first_line, &literal_key)) {\n \t\t\t/*\n \t\t\t * We only use `is_literal_ssh_key` here to check validity\n \t\t\t * The prefix will be stripped when the key is used.\n \t\t\t */\n-\t\t\tdefault_key = strbuf_detach(keys[0], NULL);\n+\t\t\tdefault_key = first_line;\n \t\t} else {\n+\t\t\tfree(first_line);\n \t\t\twarning(_(\"gpg.ssh.defaultKeyCommand succeeded but returned no keys: %s %s\"),\n \t\t\t\tkey_stderr.buf, key_stdout.buf);\n \t\t}\n \n-\t\tstrbuf_list_free(keys);\n \t} else {\n \t\twarning(_(\"gpg.ssh.defaultKeyCommand failed: %s %s\"),\n \t\t\tkey_stderr.buf, key_stdout.buf);\n-- \n2.51.0.463.g79cf913ea9\n\n"},{"id":"529219","messageId":"CAP8UFD1J_B9W62bv=0yccQNGahkv2vco3arQOs0oe0DccdTeYg@mail.gmail.com","threadId":"64360","inReplyTo":"2879d9be3659a9c1ea554fff7814507caae24b65.1760997183.git.belkid98@gmail.com","subject":"Re: [Outreachy PATCH v4 1/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-10-21T06:46:22Z","receivedAt":"2025-10-21T06:46:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Oct 21, 2025 at 12:56 AM Olamide Caleb Bello wrote:\n>\n> In get_ssh_finger_print(), the output of the `ssh-keygen` command is\n> put into `fingerprint_stdout` strbuf.\n\nNit: I think this sentence doesn't need to be in its own paragraph. It\ncould be at the start of the paragraph below.\n\n> The string in fingerprint_stdout is then split into up to 3 strbufs using\n\nNit: above the variable `fingerprint_stdout` was quoted, but now it's\nnot quoted anymore. I think it would be more consistent to quote it\nhere too.\n\n> strbuf_split_max(), however they are not modified after the split thereby\n> not making use of the strbuf API as the fingerprint token is merely\n> returned as a char * and not a strbuf, hence they do not need to be\n> strbufs.\n\nNit: this sentence is a bit long. Maybe \"however they ...\" and \"hence\nthey ...\" could start new sentences instead.\n\n> Simplify the process of retrieving and returning the desired token by\n> using strchr() to isolate the token and xmemdupz() to return a copy of the\n> token.\n> This removes the roundabout way of splitting the string into strbufs, just\n> to return the token.\n\nNit: this last sentence should either be in its own paragraph, in\nwhich case there should be a blank line before it, or it should be\npart of the previous paragraph.\n\n> Reported-by: Junio Hamano \n> Helped-by: Christian Couder \n> Helped-by: Junio Hamano \n\nNit: Junio reviews all the patches and adds his own \"Signed-off-by:\"\nto the patch that are accepted, so there is no need to also mention\nhim in an \"Helped-by:\" trailer like this.\n\n> Helped-by: Krisoffer Haughsbakk\n\nI think you mean \"Kristoffer Haugsbakk\". Please spell his name\ncorrectly and provide his email address like for everyone else.\n\n[...]\n\n> @@ -845,13 +844,17 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n> die_errno(_(\"failed to get the ssh fingerprint for key '%s'\"),\n> signing_key);\n>\n> - fingerprint = strbuf_split_max(&fingerprint_stdout, ' ', 3);\n> - if (!fingerprint[1])\n> - die_errno(_(\"failed to get the ssh fingerprint for key '%s'\"),\n> + begin = fingerprint_stdout.buf;\n\n`begin` is set here, but not used below...\n\n> + delim = strchr(fingerprint_stdout.buf, ' ');\n> + if (!delim)\n> + die_errno(_(\"failed to get the ssh fingerprint for key %s\"),\n> signing_key);\n\n(This might be an issue that already existed, but I wonder if using\ndie_errno() instead of just die() is the right thing to do here.\nShouldn't we check errno before splitting?)\n\n> - fingerprint_ret = strbuf_detach(fingerprint[1], NULL);\n> - strbuf_list_free(fingerprint);\n> + begin = delim + 1;\n\n... before here, where `begin` is set to something else. This means it\nwas useless to set it to `fingerprint_stdout.buf` before.\n\n> + delim = strchr(begin, ' ');\n> + if (!delim)\n> + die_errno(_(\"failed to get the ssh fingerprint for key %s\"),\n> + signing_key);\n> + fingerprint_ret = xmemdupz(begin, delim - begin);\n> strbuf_release(&fingerprint_stdout);\n> return fingerprint_ret;\n\nI think this could be `return xmemdupz(begin, delim - begin);`, so we\ncould get rid of `fingerprint_ret`.\n\nThanks.\n"},{"id":"529220","messageId":"CAP8UFD1=b9NN6stjnPR62Nu0qQmcC=bM2ZNQ=cO08PEwYoYAzA@mail.gmail.com","threadId":"64360","inReplyTo":"CAP8UFD1J_B9W62bv=0yccQNGahkv2vco3arQOs0oe0DccdTeYg@mail.gmail.com","subject":"Re: [Outreachy PATCH v4 1/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-10-21T06:51:21Z","receivedAt":"2025-10-21T06:51:34Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Oct 21, 2025 at 8:46 AM Christian Couder\n wrote:\n\n> > + delim = strchr(begin, ' ');\n> > + if (!delim)\n> > + die_errno(_(\"failed to get the ssh fingerprint for key %s\"),\n> > + signing_key);\n> > + fingerprint_ret = xmemdupz(begin, delim - begin);\n> > strbuf_release(&fingerprint_stdout);\n> > return fingerprint_ret;\n>\n> I think this could be `return xmemdupz(begin, delim - begin);`, so we\n> could get rid of `fingerprint_ret`.\n\nNo, actually I think we need `fingerprint_ret` because we need to call\n`xmemdupz(begin, delim - begin)` before releasing\n`fingerprint_stdout`. Sorry for the noise.\n"},{"id":"529221","messageId":"CAP8UFD1-H5jRyd6b5FhgCMLObnErXVr8p0s+kMd0qO5jWkkt2Q@mail.gmail.com","threadId":"64360","inReplyTo":"a830de15ecdb5e5f45625927cb69b2be552bda42.1760997183.git.belkid98@gmail.com","subject":"Re: [Outreachy PATCH v4 2/2] gpg-interface: do not use misdesigned strbuf_split*() [Part 2]","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-10-21T07:01:05Z","receivedAt":"2025-10-21T07:01:19Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Oct 21, 2025 at 12:57 AM Olamide Caleb Bello wrote:\n\n[...]\n\n> Reported-by: Junio Hamano \n> Helped-by: Christian Couder \n> Helped-by: Junio Hamano \n> Helped-by: Krisoffer Haughsbakk\n\nI won't repeat the issues that are the same as in patch 1/2, but\nplease correct them.\n\n[...]\n\n> @@ -887,19 +887,21 @@ static char *get_default_ssh_signing_key(void)\n> &key_stderr, 0);\n>\n> if (!ret) {\n> - keys = strbuf_split_max(&key_stdout, '\\n', 2);\n> - if (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n> + begin = key_stdout.buf;\n> + new_line = strchr(begin, '\\n');\n> + first_line = xmemdupz(begin, new_line - begin);\n\nWhat if no \\n character is found by strchr()?\n\nThanks.\n"},{"id":"529222","messageId":"CAP8UFD3sxU=r-zVmM7xL84qEsDL6cFUceAV4np6uLxFTVOnWXQ@mail.gmail.com","threadId":"64360","inReplyTo":"cover.1760997183.git.belkid98@gmail.com","subject":"Re: [Outreachy PATCH v4 0/2] do not use strbuf_split*()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-10-21T07:19:25Z","receivedAt":"2025-10-21T07:19:39Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Oct 21, 2025 at 12:56 AM Olamide Caleb Bello wrote:\n>\n> The patch series by Junio Hamano with link below,\n> https://public-inbox.org/git/20250731225433.4028872-1-gitster@poddbox.com/,\n> notices that the array of strbufs that calls to strbuf_split*() provides\n> are merely used to store the strings gotten from the split and no edit are\n> done on these resulting strings making the strbuf_split*() unideal\n> for this usecase, with the string_list_split*() being a more suitable\n> option in those cases.\n\nNow that the string_list_split*() functions are not used in your\nseries anymore, I think you can remove \"with the string_list_split*()\nbeing a more suitable option in those cases\".\n\n> Commit 2efe707054 (wt-status: avoid strbuf_split*(), 2025-07-31) for example,\n> in the series, notes that abbrev_oid_in_line() takes one line of rebase\n> todo list and splits tokens out of this line using strbuf_split_max().\n> However, no simultanous edits that take advantage of the strbuf API take\n> place but the tokens are merely used as pieces of strings.\n\nI am not sure taking this commit as an example is really useful now\nthat the string_list_split*() functions are not used in your series\nanymore. Maybe you can find a more relevant example commit in Junio's\nseries?\n\n[...]\n\n> Olamide Caleb Bello (2):\n> gpg-interface: do not use misdesigned strbuf_split*()\n> gpg-interface: do not use misdesigned strbuf_split*() [Part 2]\n\nI don't think having \"[Part 2]\" is a good idea if there is no \"[Part\n1]\". And maybe using \"part 1/2\" and \"part 2/2\" is even better if you\nwant to go this way (so that would be for example \"gpg-interface: do\nnot use misdesigned strbuf_split*(), part 1/2\"). Otherwise, I think\nit's Ok if both commits have exactly the same subject.\n\nAlso please start to use the `--in-reply-to=<...>` option of `git\nsend-email` so that your patch series are all in the same thread on\nthe mailing list archive. For example right now if you look at\nhttps://lore.kernel.org/git/cover.1760997183.git.belkid98@gmail.com/#r,\nyou will see:\n\nThread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top\n2025-10-20 22:55 Olamide Caleb Bello [this message]\n2025-10-20 22:55 ` [Outreachy PATCH v4 1/2] gpg-interface: do not use\nmisdesigned strbuf_split*() Olamide Caleb Bello\n2025-10-21 6:46 ` Christian Couder\n2025-10-21 6:51 ` Christian Couder\n2025-10-20 22:55 ` [Outreachy PATCH v4 2/2] gpg-interface: do not use\nmisdesigned strbuf_split*() [Part 2] Olamide Caleb Bello\n\nSo we don't see the previous patches and messages related to v1, v2 and v3.\n\nIf the tutorials and documentation are not clear enough, and you can't\nmake it work, then please ask for help and say what you tried so that\nwe can help you with this.\n\nThanks.\n"},{"id":"529250","messageId":"CAD=f0L-9e0uYv-T6HYkCFAWPa57y44PXV0Xi8S5MfHQVgnYUAw@mail.gmail.com","threadId":"64360","inReplyTo":"CAP8UFD3sxU=r-zVmM7xL84qEsDL6cFUceAV4np6uLxFTVOnWXQ@mail.gmail.com","subject":"Re: [Outreachy PATCH v4 0/2] do not use strbuf_split*()","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-21T10:19:53Z","receivedAt":"2025-10-21T10:20:06Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Tue, 21 Oct 2025 at 08:19, Christian Couder\n wrote:\n>\n> On Tue, Oct 21, 2025 at 12:56 AM Olamide Caleb Bello wrote:\n> >\n> > The patch series by Junio Hamano with link below,\n> > https://public-inbox.org/git/20250731225433.4028872-1-gitster@poddbox.com/,\n> > notices that the array of strbufs that calls to strbuf_split*() provides\n> > are merely used to store the strings gotten from the split and no edit are\n> > done on these resulting strings making the strbuf_split*() unideal\n> > for this usecase, with the string_list_split*() being a more suitable\n> > option in those cases.\n>\n> Now that the string_list_split*() functions are not used in your\n> series anymore, I think you can remove \"with the string_list_split*()\n> being a more suitable option in those cases\".\n\nOkay thank you.\n\n> > Commit 2efe707054 (wt-status: avoid strbuf_split*(), 2025-07-31) for example,\n> > in the series, notes that abbrev_oid_in_line() takes one line of rebase\n> > todo list and splits tokens out of this line using strbuf_split_max().\n> > However, no simultanous edits that take advantage of the strbuf API take\n> > place but the tokens are merely used as pieces of strings.\n>\n> I am not sure taking this commit as an example is really useful now\n> that the string_list_split*() functions are not used in your series\n> anymore. Maybe you can find a more relevant example commit in Junio's\n> series?\n>\n> [...]\n\nOkay. Thank you. I will take a closer look at the series and look for\na more suitable\nreference.\n\n>\n> > Olamide Caleb Bello (2):\n> > gpg-interface: do not use misdesigned strbuf_split*()\n> > gpg-interface: do not use misdesigned strbuf_split*() [Part 2]\n>\n> I don't think having \"[Part 2]\" is a good idea if there is no \"[Part\n> 1]\". And maybe using \"part 1/2\" and \"part 2/2\" is even better if you\n> want to go this way (so that would be for example \"gpg-interface: do\n> not use misdesigned strbuf_split*(), part 1/2\"). Otherwise, I think\n> it's Ok if both commits have exactly the same subject.\n\nOkay. Noted.\n>\n> Also please start to use the `--in-reply-to=<...>` option of `git\n> send-email` so that your patch series are all in the same thread on\n> the mailing list archive. For example right now if you look at\n> https://lore.kernel.org/git/cover.1760997183.git.belkid98@gmail.com/#r,\n> you will see:\n>\n> Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top\n> 2025-10-20 22:55 Olamide Caleb Bello [this message]\n> 2025-10-20 22:55 ` [Outreachy PATCH v4 1/2] gpg-interface: do not use\n> misdesigned strbuf_split*() Olamide Caleb Bello\n> 2025-10-21 6:46 ` Christian Couder\n> 2025-10-21 6:51 ` Christian Couder\n> 2025-10-20 22:55 ` [Outreachy PATCH v4 2/2] gpg-interface: do not use\n> misdesigned strbuf_split*() [Part 2] Olamide Caleb Bello\n>\n> So we don't see the previous patches and messages related to v1, v2 and v3.\n>\n> If the tutorials and documentation are not clear enough, and you can't\n> make it work, then please ask for help and say what you tried so that\n> we can help you with this.\n\nThank you very much for your time and review Christian.\n\nI will study the documentation to see how to do this.\n\nBello\n"},{"id":"529252","messageId":"CAD=f0L9A+mz=c9M_BsTLpWNAv+8wU7C+VaB42VniuiiRvgmmoQ@mail.gmail.com","threadId":"64360","inReplyTo":"CAP8UFD1J_B9W62bv=0yccQNGahkv2vco3arQOs0oe0DccdTeYg@mail.gmail.com","subject":"Re: [Outreachy PATCH v4 1/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-21T11:18:36Z","receivedAt":"2025-10-21T11:18:50Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Tue, 21 Oct 2025 at 07:46, Christian Couder\n wrote:\n>\n> On Tue, Oct 21, 2025 at 12:56 AM Olamide Caleb Bello wrote:\n> >\n> > In get_ssh_finger_print(), the output of the `ssh-keygen` command is\n> > put into `fingerprint_stdout` strbuf.\n\nOkay noted.\n>\n> Nit: I think this sentence doesn't need to be in its own paragraph. It\n> could be at the start of the paragraph below.\n>\n> > The string in fingerprint_stdout is then split into up to 3 strbufs using\n>\n> Nit: above the variable `fingerprint_stdout` was quoted, but now it's\n> not quoted anymore. I think it would be more consistent to quote it\n> here too.\n\nSorry about that. I'll fix it.\n\n>\n> > strbuf_split_max(), however they are not modified after the split thereby\n> > not making use of the strbuf API as the fingerprint token is merely\n> > returned as a char * and not a strbuf, hence they do not need to be\n> > strbufs.\n>\n> Nit: this sentence is a bit long. Maybe \"however they ...\" and \"hence\n> they ...\" could start new sentences instead.\n\nOkay thank you.\n>\n> > Simplify the process of retrieving and returning the desired token by\n> > using strchr() to isolate the token and xmemdupz() to return a copy of the\n> > token.\n> > This removes the roundabout way of splitting the string into strbufs, just\n> > to return the token.\n>\n> Nit: this last sentence should either be in its own paragraph, in\n> which case there should be a blank line before it, or it should be\n> part of the previous paragraph.\n\nOkay noted.\n>\n> > Reported-by: Junio Hamano \n> > Helped-by: Christian Couder \n> > Helped-by: Junio Hamano \n>\n> Nit: Junio reviews all the patches and adds his own \"Signed-off-by:\"\n> to the patch that are accepted, so there is no need to also mention\n> him in an \"Helped-by:\" trailer like this.\n\nOkay.\n\n>\n> > Helped-by: Krisoffer Haughsbakk\n>\n> I think you mean \"Kristoffer Haugsbakk\". Please spell his name\n> correctly and provide his email address like for everyone else.\n\nOh I'm so sorry about that his.\nI'll correct this.\nApologies.\n\n>\n> [...]\n>\n> > @@ -845,13 +844,17 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n> > die_errno(_(\"failed to get the ssh fingerprint for key '%s'\"),\n> > signing_key);\n> >\n> > - fingerprint = strbuf_split_max(&fingerprint_stdout, ' ', 3);\n> > - if (!fingerprint[1])\n> > - die_errno(_(\"failed to get the ssh fingerprint for key '%s'\"),\n> > + begin = fingerprint_stdout.buf;\n>\n> `begin` is set here, but not used below...\n>\n> > + delim = strchr(fingerprint_stdout.buf, ' ');\n\nAhh sorry I was supposed to use it here\n\n\n> > + if (!delim)\n> > + die_errno(_(\"failed to get the ssh fingerprint for key %s\"),\n> > signing_key);\n>\n> (This might be an issue that already existed, but I wonder if using\n> die_errno() instead of just die() is the right thing to do here.\n> Shouldn't we check errno before splitting?)\n\nOkay sorry I'm a bit confused.\nI should have used die() instead since we have not split the string yet?\n\n>\n> > - fingerprint_ret = strbuf_detach(fingerprint[1], NULL);\n> > - strbuf_list_free(fingerprint);\n> > + begin = delim + 1;\n>\n> ... before here, where `begin` is set to something else. This means it\n> was useless to set it to `fingerprint_stdout.buf` before.\n\nYes I should have used it in the first call to strchr ()\n\n>\n> > + delim = strchr(begin, ' ');\n> > + if (!delim)\n> > + die_errno(_(\"failed to get the ssh fingerprint for key %s\"),\n> > + signing_key);\n> > + fingerprint_ret = xmemdupz(begin, delim - begin);\n> > strbuf_release(&fingerprint_stdout);\n> > return fingerprint_ret;\n>\n> I think this could be `return xmemdupz(begin, delim - begin);`, so we\n> could get rid of `fingerprint_ret`.\n\nYes I saw your response already.\n\nThank you.\n\nApologies for resending if you're getting the mail again.\nMy first mail to the list was rejected.\n"},{"id":"529262","messageId":"CAD=f0L9sn_D437PM1LQdaY=nGCtEe0yg3Nq7_d6-yupE9pMMZg@mail.gmail.com","threadId":"64360","inReplyTo":"CAP8UFD1-H5jRyd6b5FhgCMLObnErXVr8p0s+kMd0qO5jWkkt2Q@mail.gmail.com","subject":"Re: [Outreachy PATCH v4 2/2] gpg-interface: do not use misdesigned strbuf_split*() [Part 2]","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-21T12:12:52Z","receivedAt":"2025-10-21T12:13:05Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Tue, 21 Oct 2025 at 08:01, Christian Couder\n wrote:\n>\n> On Tue, Oct 21, 2025 at 12:57 AM Olamide Caleb Bello wrote:\n>\n> [...]\n>\n> > Reported-by: Junio Hamano \n> > Helped-by: Christian Couder \n> > Helped-by: Junio Hamano \n> > Helped-by: Krisoffer Haughsbakk\n>\n> I won't repeat the issues that are the same as in patch 1/2, but\n> please correct them.\n>\n> [...]\n\nYes, thank you.\n\nI will fix them.\n>\n> > @@ -887,19 +887,21 @@ static char *get_default_ssh_signing_key(void)\n> > &key_stderr, 0);\n> >\n> > if (!ret) {\n> > - keys = strbuf_split_max(&key_stdout, '\\n', 2);\n> > - if (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n> > + begin = key_stdout.buf;\n> > + new_line = strchr(begin, '\\n');\n> > + first_line = xmemdupz(begin, new_line - begin);\n>\n> What if no \\n character is found by strchr()?\nIn the original code, just the first line of a possible two lines\nis returned.\nSo since we need just the first and if no new line is found,\nI can do\n char *end = new_line ? new_line : strchr(begin, '\\0');\n first_line = xmemdupz(begin, end);\n\nDoes this work?\n\nThanks\nBello\n"},{"id":"529303","messageId":"xmqqms5kw796.fsf@gitster.g","threadId":"64360","inReplyTo":"CAP8UFD1J_B9W62bv=0yccQNGahkv2vco3arQOs0oe0DccdTeYg@mail.gmail.com","subject":"Re: [Outreachy PATCH v4 1/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-21T16:26:45Z","receivedAt":"2025-10-21T16:26:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n>> Reported-by: Junio Hamano \n>> Helped-by: Christian Couder \n>> Helped-by: Junio Hamano \n>\n> Nit: Junio reviews all the patches and adds his own \"Signed-off-by:\"\n> to the patch that are accepted, so there is no need to also mention\n> him in an \"Helped-by:\" trailer like this.\n\nJust this point.\n\nSomebody is later expected to sign-off on the patch has little to do\nwith who is on Helped-by: lines. The provenance of whatever help by\nothers the author incorporated into the patch is covered by the\nauthor's sign-off. The sign-off I would give to this patch later is\nonly to certify that I received a signed-off patch and commited\nverbatim, or with my own changes that can be shared under the same\nDCO.\n\nNot that I think the amount of help I gave is substantial enough to\ndeserve a \"Helped-by\" credit, though.\n\nFor everything else in your review, I would very much appreciate you\nfor helping the author of the patch. Thanks.\n"},{"id":"529305","messageId":"xmqqikg8w53j.fsf@gitster.g","threadId":"64360","inReplyTo":"CAD=f0L-9e0uYv-T6HYkCFAWPa57y44PXV0Xi8S5MfHQVgnYUAw@mail.gmail.com","subject":"Re: [Outreachy PATCH v4 0/2] do not use strbuf_split*()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-21T17:13:20Z","receivedAt":"2025-10-21T17:13:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bello Olamide writes:\n\n>> > Commit 2efe707054 (wt-status: avoid strbuf_split*(), 2025-07-31) for example,\n>> > in the series, notes that abbrev_oid_in_line() takes one line of rebase\n>> > todo list and splits tokens out of this line using strbuf_split_max().\n>> > However, no simultanous edits that take advantage of the strbuf API take\n>> > place but the tokens are merely used as pieces of strings.\n>>\n>> I am not sure taking this commit as an example is really useful now\n>> that the string_list_split*() functions are not used in your series\n>> anymore. Maybe you can find a more relevant example commit in Junio's\n>> series?\n>>\n>> [...]\n>\n> Okay. Thank you. I will take a closer look at the series and look for\n> a more suitable\n> reference.\n\nThanks Christian for lending us very sharp eyes.\n\nWhat we do in these patches now is closer in spirit to d6fd08bd\n(sub-process: do not use strbuf_split*(), 2025-07-31), I think, in\nthat we do not split things into an array of strbuf, and instead\nparse things out in place as much as possible.\n"},{"id":"529397","messageId":"CAD=f0L-VOgbY+W4pNrj+JaDNm4XPQ_LnHSA0SKyaTvv2t6GP7Q@mail.gmail.com","threadId":"64360","inReplyTo":"xmqqikg8w53j.fsf@gitster.g","subject":"Re: [Outreachy PATCH v4 0/2] do not use strbuf_split*()","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-22T07:16:20Z","receivedAt":"2025-10-22T07:16:34Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Tue, 21 Oct 2025 at 18:13, Junio C Hamano wrote:\n>\n> Bello Olamide writes:\n>\n> >> > Commit 2efe707054 (wt-status: avoid strbuf_split*(), 2025-07-31) for example,\n> >> > in the series, notes that abbrev_oid_in_line() takes one line of rebase\n> >> > todo list and splits tokens out of this line using strbuf_split_max().\n> >> > However, no simultanous edits that take advantage of the strbuf API take\n> >> > place but the tokens are merely used as pieces of strings.\n> >>\n> >> I am not sure taking this commit as an example is really useful now\n> >> that the string_list_split*() functions are not used in your series\n> >> anymore. Maybe you can find a more relevant example commit in Junio's\n> >> series?\n> >>\n> >> [...]\n> >\n> > Okay. Thank you. I will take a closer look at the series and look for\n> > a more suitable\n> > reference.\n>\n> Thanks Christian for lending us very sharp eyes.\n>\n> What we do in these patches now is closer in spirit to d6fd08bd\n> (sub-process: do not use strbuf_split*(), 2025-07-31), I think, in\n> that we do not split things into an array of strbuf, and instead\n> parse things out in place as much as possible.\n\nThank you very much Junio for pointing me to a suitable reference.\nMakes my work easier :)\n\nBello\n"},{"id":"529410","messageId":"cover.1761135129.git.belkid98@gmail.com","threadId":"64360","inReplyTo":"cover.1760997183.git.belkid98@gmail.com","subject":"[Outreachy PATCH v5 0/2] do not use misdesigned strbuf_split*()","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-22T12:40:18Z","receivedAt":"2025-10-22T12:40:29Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"The patch series by Junio Hamano with link below,\nhttps://public-inbox.org/git/20250731225433.4028872-1-gitster@poddbox.com/,\nnotices that the array of strbufs that calls to strbuf_split*() provides\nare merely used to store the strings gotten from the split and no edit are\ndone on these resulting strings making the strbuf_split*() unideal\nfor this usecase.\n\nCommit d6fd08bd (sub-process: do not use strbuf_split*(), 2025-07-31) for\nexample, in the series, observes that the subprocess_read_status() reads\none packet line and tries to find \"status=\" by splitting the line\ninto two strbufs which is an overkill to extract .\n\nThis series continues on this cleanup, by replacing instances of\nstrbuf_split_max() with strchr() to get the required token around the\ndelimiter where the token from the split is merely returned as char *\nand not strbufs and no edits are done on them.\nThis makes the code cleaner, faster and more efficient.\n\nTests have also been performed on the commits on Github CI. The link is shown below\n\nhttps://github.com/git/git/pull/2080\n\nChanges in v5:\n==============\n - Modify commit messages to provide proper context for commit reference\n - Correct reviewer's name and email address\n - correct code logic by assigning `fingerprint_stdout.buf` to `begin` in\n first call to strchr in patch 1\n - Modify logic in call to strchr() when no '\\n' is found by passing '\\0'\n to get the end of first line.\n - use die() in place of die_errno() in failed calls to strchr(), retaining\n die_errno() before the string is split\n\n\nOlamide Caleb Bello (2):\n gpg-interface: do not use misdesigned strbuf_split*()\n gpg-interface: do not use misdesigned strbuf_split*()\n\n gpg-interface.c | 32 +++++++++++++++++++-------------\n 1 file changed, 19 insertions(+), 13 deletions(-)\n\nRange diff versus v4\n====================\n\n1: 2879d9be36 ! 1: df8fbbd3a5 gpg-interface: do not use misdesigned strbuf_split*()\n @@ Commit message\n \n In get_ssh_finger_print(), the output of the `ssh-keygen` command is\n put into `fingerprint_stdout` strbuf.\n -\n - The string in fingerprint_stdout is then split into up to 3 strbufs using\n - strbuf_split_max(), however they are not modified after the split thereby\n - not making use of the strbuf API as the fingerprint token is merely\n - returned as a char * and not a strbuf, hence they do not need to be\n + The string in `fingerprint_stdout` is then split into up to 3 strbufs\n + using strbuf_split_max(). However they are not modified after the split\n + thereby not making use of the strbuf API as the fingerprint token is\n + merely returned as a char * and not a strbuf. Hence they do not need to be\n strbufs.\n \n Simplify the process of retrieving and returning the desired token by\n using strchr() to isolate the token and xmemdupz() to return a copy of the\n - token.\n - This removes the roundabout way of splitting the string into strbufs, just\n - to return the token.\n + token. This removes the roundabout way of splitting the string into\n + strbufs just to return the token.\n \n Reported-by: Junio Hamano \n Helped-by: Christian Couder \n - Helped-by: Junio Hamano \n - Helped-by: Krisoffer Haughsbakk\n + Helped-by: Kristoffer Haugsbakk \n Signed-off-by: Olamide Caleb Bello \n \n ## gpg-interface.c ##\n @@ gpg-interface.c: static char *get_ssh_key_fingerprint(const char *signing_key)\n -\tif (!fingerprint[1])\n -\t\tdie_errno(_(\"failed to get the ssh fingerprint for key '%s'\"),\n +\tbegin = fingerprint_stdout.buf;\n -+\tdelim = strchr(fingerprint_stdout.buf, ' ');\n ++\tdelim = strchr(begin, ' ');\n +\tif (!delim)\n -+\t\tdie_errno(_(\"failed to get the ssh fingerprint for key %s\"),\n ++\t\tdie(_(\"failed to get the ssh fingerprint for key %s\"),\n \t\t\t signing_key);\n -\n -\tfingerprint_ret = strbuf_detach(fingerprint[1], NULL);\n @@ gpg-interface.c: static char *get_ssh_key_fingerprint(const char *signing_key)\n +\tbegin = delim + 1;\n +\tdelim = strchr(begin, ' ');\n +\tif (!delim)\n -+\t die_errno(_(\"failed to get the ssh fingerprint for key %s\"),\n ++\t die(_(\"failed to get the ssh fingerprint for key %s\"),\n +\t\t\t signing_key);\n +\tfingerprint_ret = xmemdupz(begin, delim - begin);\n \tstrbuf_release(&fingerprint_stdout);\n2: a830de15ec ! 2: 5df667227b gpg-interface: do not use misdesigned strbuf_split*() [Part 2]\n @@ Metadata\n Author: Olamide Caleb Bello \n \n ## Commit message ##\n - gpg-interface: do not use misdesigned strbuf_split*() [Part 2]\n + gpg-interface: do not use misdesigned strbuf_split*()\n \n In get_default_ssh_signing_key(), the default ssh signing key is\n retrieved in `key_stdout` buf, which is then split using\n @@ Commit message\n \n Reported-by: Junio Hamano \n Helped-by: Christian Couder \n - Helped-by: Junio Hamano \n - Helped-by: Krisoffer Haughsbakk\n + Helped-by: Kristoffer Haugsbakk \n Signed-off-by: Olamide Caleb Bello \n \n ## gpg-interface.c ##\n @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)\n \tint n;\n \tchar *default_key = NULL;\n \tconst char *literal_key = NULL;\n -+\tchar *begin, *new_line, *first_line;\n ++\tchar *begin, *new_line, *first_line, *end;\n \n \tif (!ssh_default_key_command)\n \t\tdie(_(\"either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured\"));\n @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)\n -\t\tif (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n +\t\tbegin = key_stdout.buf;\n +\t\tnew_line = strchr(begin, '\\n');\n -+\t\tfirst_line = xmemdupz(begin, new_line - begin);\n ++\t\tend = new_line ? new_line : strchr(begin, '\\0');\n ++\t\tfirst_line = xmemdupz(begin, end - begin);\n +\t\tif (is_literal_ssh_key(first_line, &literal_key)) {\n \t\t\t/*\n \t\t\t * We only use `is_literal_ssh_key` here to check validity\n\n-- \n2.51.0.463.g79cf913ea9\n\n"},{"id":"529411","messageId":"df8fbbd3a50748fd974083b6bbb07ffca91be465.1761135129.git.belkid98@gmail.com","threadId":"64360","inReplyTo":"cover.1761135129.git.belkid98@gmail.com","subject":"[Outreachy PATCH v5 1/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-22T12:40:19Z","receivedAt":"2025-10-22T12:40:33Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"In get_ssh_finger_print(), the output of the `ssh-keygen` command is\nput into `fingerprint_stdout` strbuf.\nThe string in `fingerprint_stdout` is then split into up to 3 strbufs\nusing strbuf_split_max(). However they are not modified after the split\nthereby not making use of the strbuf API as the fingerprint token is\nmerely returned as a char * and not a strbuf. Hence they do not need to be\nstrbufs.\n\nSimplify the process of retrieving and returning the desired token by\nusing strchr() to isolate the token and xmemdupz() to return a copy of the\ntoken. This removes the roundabout way of splitting the string into\nstrbufs just to return the token.\n\nReported-by: Junio Hamano \nHelped-by: Christian Couder \nHelped-by: Kristoffer Haugsbakk \nSigned-off-by: Olamide Caleb Bello \n---\n gpg-interface.c | 19 +++++++++++--------\n 1 file changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 2f4f0e32cb..917081abac 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -821,8 +821,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n \tstruct child_process ssh_keygen = CHILD_PROCESS_INIT;\n \tint ret = -1;\n \tstruct strbuf fingerprint_stdout = STRBUF_INIT;\n-\tstruct strbuf **fingerprint;\n-\tchar *fingerprint_ret;\n+\tchar *fingerprint_ret, *begin, *delim;\n \tconst char *literal_key = NULL;\n \n \t/*\n@@ -845,13 +844,17 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n \t\tdie_errno(_(\"failed to get the ssh fingerprint for key '%s'\"),\n \t\t\t signing_key);\n \n-\tfingerprint = strbuf_split_max(&fingerprint_stdout, ' ', 3);\n-\tif (!fingerprint[1])\n-\t\tdie_errno(_(\"failed to get the ssh fingerprint for key '%s'\"),\n+\tbegin = fingerprint_stdout.buf;\n+\tdelim = strchr(begin, ' ');\n+\tif (!delim)\n+\t\tdie(_(\"failed to get the ssh fingerprint for key %s\"),\n \t\t\t signing_key);\n-\n-\tfingerprint_ret = strbuf_detach(fingerprint[1], NULL);\n-\tstrbuf_list_free(fingerprint);\n+\tbegin = delim + 1;\n+\tdelim = strchr(begin, ' ');\n+\tif (!delim)\n+\t die(_(\"failed to get the ssh fingerprint for key %s\"),\n+\t\t\t signing_key);\n+\tfingerprint_ret = xmemdupz(begin, delim - begin);\n \tstrbuf_release(&fingerprint_stdout);\n \treturn fingerprint_ret;\n }\n-- \n2.51.0.463.g79cf913ea9\n\n"},{"id":"529412","messageId":"5df667227b8b8951bad6c3cba54230ea8f6d3830.1761135129.git.belkid98@gmail.com","threadId":"64360","inReplyTo":"cover.1761135129.git.belkid98@gmail.com","subject":"[Outreachy PATCH v5 2/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-22T12:40:20Z","receivedAt":"2025-10-22T12:40:36Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"In get_default_ssh_signing_key(), the default ssh signing key is\nretrieved in `key_stdout` buf, which is then split using\nstrbuf_split_max() into up to two strbufs at a new line and the first\nstrbuf is returned as a `char *`and not a strbuf.\nThis makes the function lack the use of strbuf API as no edits are\nperformed on the split tokens.\n\nSimplify the process of retrieving and returning the desired line by\nusing strchr() to isolate the line and xmemdupz() to return a copy of the\nline.\nThis removes the roundabout way of splitting the string into strbufs, just\nto return the line.\n\nReported-by: Junio Hamano \nHelped-by: Christian Couder \nHelped-by: Kristoffer Haugsbakk \nSigned-off-by: Olamide Caleb Bello \n---\n gpg-interface.c | 13 ++++++++-----\n 1 file changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 917081abac..ad6ce58da8 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -865,12 +865,12 @@ static char *get_default_ssh_signing_key(void)\n \tstruct child_process ssh_default_key = CHILD_PROCESS_INIT;\n \tint ret = -1;\n \tstruct strbuf key_stdout = STRBUF_INIT, key_stderr = STRBUF_INIT;\n-\tstruct strbuf **keys;\n \tchar *key_command = NULL;\n \tconst char **argv;\n \tint n;\n \tchar *default_key = NULL;\n \tconst char *literal_key = NULL;\n+\tchar *begin, *new_line, *first_line, *end;\n \n \tif (!ssh_default_key_command)\n \t\tdie(_(\"either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured\"));\n@@ -887,19 +887,22 @@ static char *get_default_ssh_signing_key(void)\n \t\t\t &key_stderr, 0);\n \n \tif (!ret) {\n-\t\tkeys = strbuf_split_max(&key_stdout, '\\n', 2);\n-\t\tif (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n+\t\tbegin = key_stdout.buf;\n+\t\tnew_line = strchr(begin, '\\n');\n+\t\tend = new_line ? new_line : strchr(begin, '\\0');\n+\t\tfirst_line = xmemdupz(begin, end - begin);\n+\t\tif (is_literal_ssh_key(first_line, &literal_key)) {\n \t\t\t/*\n \t\t\t * We only use `is_literal_ssh_key` here to check validity\n \t\t\t * The prefix will be stripped when the key is used.\n \t\t\t */\n-\t\t\tdefault_key = strbuf_detach(keys[0], NULL);\n+\t\t\tdefault_key = first_line;\n \t\t} else {\n+\t\t\tfree(first_line);\n \t\t\twarning(_(\"gpg.ssh.defaultKeyCommand succeeded but returned no keys: %s %s\"),\n \t\t\t\tkey_stderr.buf, key_stdout.buf);\n \t\t}\n \n-\t\tstrbuf_list_free(keys);\n \t} else {\n \t\twarning(_(\"gpg.ssh.defaultKeyCommand failed: %s %s\"),\n \t\t\tkey_stderr.buf, key_stdout.buf);\n-- \n2.51.0.463.g79cf913ea9\n\n"},{"id":"529417","messageId":"CAP8UFD2GCG5y7c=utQ43M=TfVPSDF0qUUAXH+U2nRpeuKfcW=w@mail.gmail.com","threadId":"64360","inReplyTo":"df8fbbd3a50748fd974083b6bbb07ffca91be465.1761135129.git.belkid98@gmail.com","subject":"Re: [Outreachy PATCH v5 1/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-10-22T13:56:34Z","receivedAt":"2025-10-22T13:56:48Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Oct 22, 2025 at 2:40 PM Olamide Caleb Bello wrote:\n>\n> In get_ssh_finger_print(), the output of the `ssh-keygen` command is\n> put into `fingerprint_stdout` strbuf.\n> The string in `fingerprint_stdout` is then split into up to 3 strbufs\n\nNit: it's not clear if the first sentence of this commit message is\npart of the same paragraph as the second sentence or not. If you\nreroll this patch, I would suggest making it clearly part of the same\nparagraph like this:\n\n\"In get_ssh_finger_print(), the output of the `ssh-keygen` command is\nput into `fingerprint_stdout` strbuf. The string in `fingerprint_stdout` is\nthen split into up to 3 strbufs using strbuf_split_max(). However...\"\n\nOtherwise this patch looks fine to me.\n\nThanks.\n"},{"id":"529418","messageId":"CAP8UFD3OTMi6uxv+z4rTqJ4wVpmezSG2Yj8tZMpgptWaWU343w@mail.gmail.com","threadId":"64360","inReplyTo":"5df667227b8b8951bad6c3cba54230ea8f6d3830.1761135129.git.belkid98@gmail.com","subject":"Re: [Outreachy PATCH v5 2/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-10-22T14:03:50Z","receivedAt":"2025-10-22T14:04:04Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Oct 22, 2025 at 2:40 PM Olamide Caleb Bello wrote:\n\n[...]\n\n> Simplify the process of retrieving and returning the desired line by\n> using strchr() to isolate the line and xmemdupz() to return a copy of the\n> line.\n> This removes the roundabout way of splitting the string into strbufs, just\n> to return the line.\n\nNit: here also I think it should be clear that these last two\nsentences are in the same paragraph.\n\n[...]\n\n> @@ -887,19 +887,22 @@ static char *get_default_ssh_signing_key(void)\n> &key_stderr, 0);\n>\n> if (!ret) {\n> - keys = strbuf_split_max(&key_stdout, '\\n', 2);\n> - if (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n> + begin = key_stdout.buf;\n> + new_line = strchr(begin, '\\n');\n> + end = new_line ? new_line : strchr(begin, '\\0');\n> + first_line = xmemdupz(begin, end - begin);\n\nThat works but I wonder if something like the following is not a bit better:\n\n if (new_line)\n first_line = xmemdupz(begin, new_line - begin);\n else\n first_line = xstrdup(begin);\n\nThanks.\n"},{"id":"529432","messageId":"xmqq4irqzv9y.fsf@gitster.g","threadId":"64360","inReplyTo":"CAP8UFD3OTMi6uxv+z4rTqJ4wVpmezSG2Yj8tZMpgptWaWU343w@mail.gmail.com","subject":"Re: [Outreachy PATCH v5 2/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-22T17:44:09Z","receivedAt":"2025-10-22T17:44:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n>> @@ -887,19 +887,22 @@ static char *get_default_ssh_signing_key(void)\n>> &key_stderr, 0);\n>>\n>> if (!ret) {\n>> - keys = strbuf_split_max(&key_stdout, '\\n', 2);\n>> - if (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n>> + begin = key_stdout.buf;\n>> + new_line = strchr(begin, '\\n');\n>> + end = new_line ? new_line : strchr(begin, '\\0');\n>> + first_line = xmemdupz(begin, end - begin);\n>\n> That works but I wonder if something like the following is not a bit better:\n>\n> if (new_line)\n> first_line = xmemdupz(begin, new_line - begin);\n> else\n> first_line = xstrdup(begin);\n\nYeah, that is certainly much easier to understand without even\nreading and thinking.\n\nThanks.\n"},{"id":"529489","messageId":"CAD=f0L-pmB22DpK7kDr7Oe4iztPeHgbserTqn3=icYVvryVx9w@mail.gmail.com","threadId":"64360","inReplyTo":"CAP8UFD2GCG5y7c=utQ43M=TfVPSDF0qUUAXH+U2nRpeuKfcW=w@mail.gmail.com","subject":"Re: [Outreachy PATCH v5 1/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-23T08:14:20Z","receivedAt":"2025-10-23T08:14:19Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Wed, 22 Oct 2025 at 14:56, Christian Couder\n wrote:\n>\n> On Wed, Oct 22, 2025 at 2:40 PM Olamide Caleb Bello wrote:\n> >\n> > In get_ssh_finger_print(), the output of the `ssh-keygen` command is\n> > put into `fingerprint_stdout` strbuf.\n> > The string in `fingerprint_stdout` is then split into up to 3 strbufs\n>\n> Nit: it's not clear if the first sentence of this commit message is\n> part of the same paragraph as the second sentence or not. If you\n> reroll this patch, I would suggest making it clearly part of the same\n> paragraph like this:\n>\n> \"In get_ssh_finger_print(), the output of the `ssh-keygen` command is\n> put into `fingerprint_stdout` strbuf. The string in `fingerprint_stdout` is\n> then split into up to 3 strbufs using strbuf_split_max(). However...\"\n>\n> Otherwise this patch looks fine to me.\n>\n> Thanks.\n\nOkay thank you very much\n\nBello\n"},{"id":"529490","messageId":"CAD=f0L9qsVOo5=2XfKxd-UvzzzJ=PEE0-kW=wyOxSVvTYt7Vyw@mail.gmail.com","threadId":"64360","inReplyTo":"CAP8UFD3OTMi6uxv+z4rTqJ4wVpmezSG2Yj8tZMpgptWaWU343w@mail.gmail.com","subject":"Re: [Outreachy PATCH v5 2/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Bello Olamide","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-23T08:17:31Z","receivedAt":"2025-10-23T08:17:30Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"On Wed, 22 Oct 2025 at 15:04, Christian Couder\n wrote:\n>\n> On Wed, Oct 22, 2025 at 2:40 PM Olamide Caleb Bello wrote:\n>\n> [...]\n>\n> > Simplify the process of retrieving and returning the desired line by\n> > using strchr() to isolate the line and xmemdupz() to return a copy of the\n> > line.\n> > This removes the roundabout way of splitting the string into strbufs, just\n> > to return the line.\n>\n> Nit: here also I think it should be clear that these last two\n> sentences are in the same paragraph.\n\nOkay\n\n>\n> [...]\n>\n> > @@ -887,19 +887,22 @@ static char *get_default_ssh_signing_key(void)\n> > &key_stderr, 0);\n> >\n> > if (!ret) {\n> > - keys = strbuf_split_max(&key_stdout, '\\n', 2);\n> > - if (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n> > + begin = key_stdout.buf;\n> > + new_line = strchr(begin, '\\n');\n> > + end = new_line ? new_line : strchr(begin, '\\0');\n> > + first_line = xmemdupz(begin, end - begin);\n>\n> That works but I wonder if something like the following is not a bit better:\n>\n> if (new_line)\n> first_line = xmemdupz(begin, new_line - begin);\n> else\n> first_line = xstrdup(begin);\n\nAh yes.\nIt is much better.\nThank you very much for your guide.\nI have already learnt a lot in this series.\n\nBello\n"},{"id":"529492","messageId":"cover.1761217100.git.belkid98@gmail.com","threadId":"64360","inReplyTo":"cover.1761135129.git.belkid98@gmail.com","subject":"[Outreachy PATCH v6 0/2] do not use misdesigned strbuf_split*()","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-23T11:13:45Z","receivedAt":"2025-10-23T11:13:55Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"The patch series by Junio Hamano with link below,\nhttps://public-inbox.org/git/20250731225433.4028872-1-gitster@poddbox.com/,\nnotices that the array of strbufs that calls to strbuf_split*() provides\nare merely used to store the strings gotten from the split and no edit are\ndone on these resulting strings making the strbuf_split*() unideal\nfor this usecase.\n\nCommit d6fd08bd (sub-process: do not use strbuf_split*(), 2025-07-31) for\nexample, in the series, observes that the subprocess_read_status() reads\none packet line and tries to find \"status=\" by splitting the line\ninto two strbufs which is an overkill to extract .\n\nThis series continues on this cleanup, by replacing instances of\nstrbuf_split_max() with strchr() to get the required token around the\ndelimiter where the token from the split is merely returned as char *\nand not strbufs and no edits are done on them.\nThis makes the code cleaner, faster and more efficient.\n\nTests have also been performed on the commits on Github CI. The link is shown below\n\nhttps://github.com/git/git/pull/2080\n\nChanges in v6\n=============\n- Modify commit messages to have proper structure\n- Changed logic in get_default_ssh_signing_key() to use xmemdupz() if\n key has '\\n' and xstrdup() if not.\n\nOlamide Caleb Bello (2):\n gpg-interface: do not use misdesigned strbuf_split*()\n gpg-interface: do not use misdesigned strbuf_split*()\n\n gpg-interface.c | 34 +++++++++++++++++++++-------------\n 1 file changed, 21 insertions(+), 13 deletions(-)\n\nRange diff versus v5\n====================\n\n1: df8fbbd3a5 ! 1: 92fc78c203 gpg-interface: do not use misdesigned strbuf_split*()\n @@ Commit message\n gpg-interface: do not use misdesigned strbuf_split*()\n\n In get_ssh_finger_print(), the output of the `ssh-keygen` command is\n - put into `fingerprint_stdout` strbuf.\n - The string in `fingerprint_stdout` is then split into up to 3 strbufs\n - using strbuf_split_max(). However they are not modified after the split\n - thereby not making use of the strbuf API as the fingerprint token is\n - merely returned as a char * and not a strbuf. Hence they do not need to be\n - strbufs.\n + put into `fingerprint_stdout` strbuf. The string in `fingerprint_stdout`\n + is then split into up to 3 strbufs using strbuf_split_max(). However they\n + are not modified after the split thereby not making use of the strbuf API\n + as the fingerprint token is merely returned as a char * and not a strbuf.\n + Hence they do not need to be strbufs.\n\n Simplify the process of retrieving and returning the desired token by\n using strchr() to isolate the token and xmemdupz() to return a copy of the\n2: 5df667227b ! 2: e52855242c gpg-interface: do not use misdesigned strbuf_split*()\n @@ Commit message\n\n Simplify the process of retrieving and returning the desired line by\n using strchr() to isolate the line and xmemdupz() to return a copy of the\n - line.\n - This removes the roundabout way of splitting the string into strbufs, just\n - to return the line.\n + line. This removes the roundabout way of splitting the string into\n + strbufs, just to return the line.\n\n Reported-by: Junio Hamano \n Helped-by: Christian Couder \n @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)\n \tint n;\n \tchar *default_key = NULL;\n \tconst char *literal_key = NULL;\n -+\tchar *begin, *new_line, *first_line, *end;\n ++\tchar *begin, *new_line, *first_line;\n\n \tif (!ssh_default_key_command)\n \t\tdie(_(\"either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured\"));\n @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)\n -\t\tif (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n +\t\tbegin = key_stdout.buf;\n +\t\tnew_line = strchr(begin, '\\n');\n -+\t\tend = new_line ? new_line : strchr(begin, '\\0');\n -+\t\tfirst_line = xmemdupz(begin, end - begin);\n ++\t\tif (new_line)\n ++\t\t\tfirst_line = xmemdupz(begin, new_line - begin);\n ++\t\telse\n ++\t\t\tfirst_line = xstrdup(begin);\n +\t\tif (is_literal_ssh_key(first_line, &literal_key)) {\n \t\t\t/*\n \t\t\t * We only use `is_literal_ssh_key` here to check validity\n\n--\n2.51.0.463.g79cf913ea9\n\n"},{"id":"529493","messageId":"92fc78c203646fe30155fefe2fd041f99bde1b7c.1761217100.git.belkid98@gmail.com","threadId":"64360","inReplyTo":"cover.1761217100.git.belkid98@gmail.com","subject":"[Outreachy PATCH v6 1/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-23T11:13:46Z","receivedAt":"2025-10-23T11:13:59Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"In get_ssh_finger_print(), the output of the `ssh-keygen` command is\nput into `fingerprint_stdout` strbuf. The string in `fingerprint_stdout`\nis then split into up to 3 strbufs using strbuf_split_max(). However they\nare not modified after the split thereby not making use of the strbuf API\nas the fingerprint token is merely returned as a char * and not a strbuf.\nHence they do not need to be strbufs.\n\nSimplify the process of retrieving and returning the desired token by\nusing strchr() to isolate the token and xmemdupz() to return a copy of the\ntoken. This removes the roundabout way of splitting the string into\nstrbufs just to return the token.\n\nReported-by: Junio Hamano \nHelped-by: Christian Couder \nHelped-by: Kristoffer Haugsbakk \nSigned-off-by: Olamide Caleb Bello \n---\n gpg-interface.c | 19 +++++++++++--------\n 1 file changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 2f4f0e32cb..917081abac 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -821,8 +821,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n \tstruct child_process ssh_keygen = CHILD_PROCESS_INIT;\n \tint ret = -1;\n \tstruct strbuf fingerprint_stdout = STRBUF_INIT;\n-\tstruct strbuf **fingerprint;\n-\tchar *fingerprint_ret;\n+\tchar *fingerprint_ret, *begin, *delim;\n \tconst char *literal_key = NULL;\n \n \t/*\n@@ -845,13 +844,17 @@ static char *get_ssh_key_fingerprint(const char *signing_key)\n \t\tdie_errno(_(\"failed to get the ssh fingerprint for key '%s'\"),\n \t\t\t signing_key);\n \n-\tfingerprint = strbuf_split_max(&fingerprint_stdout, ' ', 3);\n-\tif (!fingerprint[1])\n-\t\tdie_errno(_(\"failed to get the ssh fingerprint for key '%s'\"),\n+\tbegin = fingerprint_stdout.buf;\n+\tdelim = strchr(begin, ' ');\n+\tif (!delim)\n+\t\tdie(_(\"failed to get the ssh fingerprint for key %s\"),\n \t\t\t signing_key);\n-\n-\tfingerprint_ret = strbuf_detach(fingerprint[1], NULL);\n-\tstrbuf_list_free(fingerprint);\n+\tbegin = delim + 1;\n+\tdelim = strchr(begin, ' ');\n+\tif (!delim)\n+\t die(_(\"failed to get the ssh fingerprint for key %s\"),\n+\t\t\t signing_key);\n+\tfingerprint_ret = xmemdupz(begin, delim - begin);\n \tstrbuf_release(&fingerprint_stdout);\n \treturn fingerprint_ret;\n }\n-- \n2.51.0.463.g79cf913ea9\n\n"},{"id":"529494","messageId":"e52855242c8f297f709bb3e5998c25ce9c4f3ac6.1761217100.git.belkid98@gmail.com","threadId":"64360","inReplyTo":"cover.1761217100.git.belkid98@gmail.com","subject":"[Outreachy PATCH v6 2/2] gpg-interface: do not use misdesigned strbuf_split*()","fromName":"Olamide Caleb Bello","fromEmail":"belkid98@gmail.com","sentAt":"2025-10-23T11:13:47Z","receivedAt":"2025-10-23T11:14:04Z","isPatch":true,"sender":{"key":"belkid98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/73387291?v=4"},"body":"In get_default_ssh_signing_key(), the default ssh signing key is\nretrieved in `key_stdout` buf, which is then split using\nstrbuf_split_max() into up to two strbufs at a new line and the first\nstrbuf is returned as a `char *`and not a strbuf.\nThis makes the function lack the use of strbuf API as no edits are\nperformed on the split tokens.\n\nSimplify the process of retrieving and returning the desired line by\nusing strchr() to isolate the line and xmemdupz() to return a copy of the\nline. This removes the roundabout way of splitting the string into\nstrbufs, just to return the line.\n\nReported-by: Junio Hamano \nHelped-by: Christian Couder \nHelped-by: Kristoffer Haugsbakk \nSigned-off-by: Olamide Caleb Bello \n---\n gpg-interface.c | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 917081abac..d1e88da8c1 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -865,12 +865,12 @@ static char *get_default_ssh_signing_key(void)\n \tstruct child_process ssh_default_key = CHILD_PROCESS_INIT;\n \tint ret = -1;\n \tstruct strbuf key_stdout = STRBUF_INIT, key_stderr = STRBUF_INIT;\n-\tstruct strbuf **keys;\n \tchar *key_command = NULL;\n \tconst char **argv;\n \tint n;\n \tchar *default_key = NULL;\n \tconst char *literal_key = NULL;\n+\tchar *begin, *new_line, *first_line;\n \n \tif (!ssh_default_key_command)\n \t\tdie(_(\"either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured\"));\n@@ -887,19 +887,24 @@ static char *get_default_ssh_signing_key(void)\n \t\t\t &key_stderr, 0);\n \n \tif (!ret) {\n-\t\tkeys = strbuf_split_max(&key_stdout, '\\n', 2);\n-\t\tif (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n+\t\tbegin = key_stdout.buf;\n+\t\tnew_line = strchr(begin, '\\n');\n+\t\tif (new_line)\n+\t\t\tfirst_line = xmemdupz(begin, new_line - begin);\n+\t\telse\n+\t\t\tfirst_line = xstrdup(begin);\n+\t\tif (is_literal_ssh_key(first_line, &literal_key)) {\n \t\t\t/*\n \t\t\t * We only use `is_literal_ssh_key` here to check validity\n \t\t\t * The prefix will be stripped when the key is used.\n \t\t\t */\n-\t\t\tdefault_key = strbuf_detach(keys[0], NULL);\n+\t\t\tdefault_key = first_line;\n \t\t} else {\n+\t\t\tfree(first_line);\n \t\t\twarning(_(\"gpg.ssh.defaultKeyCommand succeeded but returned no keys: %s %s\"),\n \t\t\t\tkey_stderr.buf, key_stdout.buf);\n \t\t}\n \n-\t\tstrbuf_list_free(keys);\n \t} else {\n \t\twarning(_(\"gpg.ssh.defaultKeyCommand failed: %s %s\"),\n \t\t\tkey_stderr.buf, key_stdout.buf);\n-- \n2.51.0.463.g79cf913ea9\n\n"},{"id":"529509","messageId":"xmqqecqtwpl5.fsf@gitster.g","threadId":"64360","inReplyTo":"cover.1761217100.git.belkid98@gmail.com","subject":"Re: [Outreachy PATCH v6 0/2] do not use misdesigned strbuf_split*()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-23T16:27:34Z","receivedAt":"2025-10-23T16:27:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Olamide Caleb Bello writes:\n\n> Changes in v6\n> =============\n> - Modify commit messages to have proper structure\n> - Changed logic in get_default_ssh_signing_key() to use xmemdupz() if\n> key has '\\n' and xstrdup() if not.\n\nThis round looks good to me. Christian, should we declare victory\nand mark it for 'next' now?\n\nThanks.\n\n>\n> Olamide Caleb Bello (2):\n> gpg-interface: do not use misdesigned strbuf_split*()\n> gpg-interface: do not use misdesigned strbuf_split*()\n>\n> gpg-interface.c | 34 +++++++++++++++++++++-------------\n> 1 file changed, 21 insertions(+), 13 deletions(-)\n>\n> Range diff versus v5\n> ====================\n>\n> 1: df8fbbd3a5 ! 1: 92fc78c203 gpg-interface: do not use misdesigned strbuf_split*()\n> @@ Commit message\n> gpg-interface: do not use misdesigned strbuf_split*()\n>\n> In get_ssh_finger_print(), the output of the `ssh-keygen` command is\n> - put into `fingerprint_stdout` strbuf.\n> - The string in `fingerprint_stdout` is then split into up to 3 strbufs\n> - using strbuf_split_max(). However they are not modified after the split\n> - thereby not making use of the strbuf API as the fingerprint token is\n> - merely returned as a char * and not a strbuf. Hence they do not need to be\n> - strbufs.\n> + put into `fingerprint_stdout` strbuf. The string in `fingerprint_stdout`\n> + is then split into up to 3 strbufs using strbuf_split_max(). However they\n> + are not modified after the split thereby not making use of the strbuf API\n> + as the fingerprint token is merely returned as a char * and not a strbuf.\n> + Hence they do not need to be strbufs.\n>\n> Simplify the process of retrieving and returning the desired token by\n> using strchr() to isolate the token and xmemdupz() to return a copy of the\n> 2: 5df667227b ! 2: e52855242c gpg-interface: do not use misdesigned strbuf_split*()\n> @@ Commit message\n>\n> Simplify the process of retrieving and returning the desired line by\n> using strchr() to isolate the line and xmemdupz() to return a copy of the\n> - line.\n> - This removes the roundabout way of splitting the string into strbufs, just\n> - to return the line.\n> + line. This removes the roundabout way of splitting the string into\n> + strbufs, just to return the line.\n>\n> Reported-by: Junio Hamano \n> Helped-by: Christian Couder \n> @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)\n> \tint n;\n> \tchar *default_key = NULL;\n> \tconst char *literal_key = NULL;\n> -+\tchar *begin, *new_line, *first_line, *end;\n> ++\tchar *begin, *new_line, *first_line;\n>\n> \tif (!ssh_default_key_command)\n> \t\tdie(_(\"either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured\"));\n> @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)\n> -\t\tif (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {\n> +\t\tbegin = key_stdout.buf;\n> +\t\tnew_line = strchr(begin, '\\n');\n> -+\t\tend = new_line ? new_line : strchr(begin, '\\0');\n> -+\t\tfirst_line = xmemdupz(begin, end - begin);\n> ++\t\tif (new_line)\n> ++\t\t\tfirst_line = xmemdupz(begin, new_line - begin);\n> ++\t\telse\n> ++\t\t\tfirst_line = xstrdup(begin);\n> +\t\tif (is_literal_ssh_key(first_line, &literal_key)) {\n> \t\t\t/*\n> \t\t\t * We only use `is_literal_ssh_key` here to check validity\n>\n> --\n> 2.51.0.463.g79cf913ea9\n"},{"id":"529600","messageId":"CAP8UFD1fousSKKduFAaZrsV9REnOaRDOQYcqB+rTQ0Ys60OWGA@mail.gmail.com","threadId":"64360","inReplyTo":"xmqqecqtwpl5.fsf@gitster.g","subject":"Re: [Outreachy PATCH v6 0/2] do not use misdesigned strbuf_split*()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-10-24T13:25:08Z","receivedAt":"2025-10-24T13:25:22Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Oct 23, 2025 at 6:27 PM Junio C Hamano wrote:\n>\n> Olamide Caleb Bello writes:\n>\n> > Changes in v6\n> > =============\n> > - Modify commit messages to have proper structure\n> > - Changed logic in get_default_ssh_signing_key() to use xmemdupz() if\n> > key has '\\n' and xstrdup() if not.\n>\n> This round looks good to me. Christian, should we declare victory\n> and mark it for 'next' now?\n\nYeah, v6 looks good to me too. Acked.\n\nThanks.\n"}]}