git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Olamide Caleb Bello <belkid98@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, usmanakinyemi202@gmail.com,
	christian.couder@gmail.com, kristofferhaugsbakk@fastmail.com,
	Olamide Caleb Bello <belkid98@gmail.com>
Subject: [Outreachy PATCH v6 0/2] do not use misdesigned strbuf_split*()
Date: Thu, 23 Oct 2025 11:13:45 +0000	[thread overview]
Message-ID: <cover.1761217100.git.belkid98@gmail.com> (raw)
In-Reply-To: <cover.1761135129.git.belkid98@gmail.com>

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

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

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

Tests have also been performed on the commits on Github CI. The link is shown below

https://github.com/git/git/pull/2080

Changes in v6
=============
- Modify commit messages to have proper structure
- Changed logic in get_default_ssh_signing_key() to use xmemdupz() if
  key has '\n' and xstrdup() if not.

Olamide Caleb Bello (2):
  gpg-interface: do not use misdesigned strbuf_split*()
  gpg-interface: do not use misdesigned strbuf_split*()

 gpg-interface.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

Range diff versus v5
====================

1:  df8fbbd3a5 ! 1:  92fc78c203 gpg-interface: do not use misdesigned strbuf_split*()
    @@ Commit message
         gpg-interface: do not use misdesigned strbuf_split*()

         In get_ssh_finger_print(), the output of the `ssh-keygen` command is
    -    put into `fingerprint_stdout` strbuf.
    -    The string in `fingerprint_stdout` is then split into up to 3 strbufs
    -    using strbuf_split_max(). However they are not modified after the split
    -    thereby not making use of the strbuf API as the fingerprint token is
    -    merely returned as a char * and not a strbuf. Hence they do not need to be
    -    strbufs.
    +    put into `fingerprint_stdout` strbuf. The string in `fingerprint_stdout`
    +    is then split into up to 3 strbufs using strbuf_split_max(). However they
    +    are not modified after the split thereby not making use of the strbuf API
    +    as the fingerprint token is merely returned as a char * and not a strbuf.
    +    Hence they do not need to be strbufs.

         Simplify the process of retrieving and returning the desired token by
         using strchr() to isolate the token and xmemdupz() to return a copy of the
2:  5df667227b ! 2:  e52855242c gpg-interface: do not use misdesigned strbuf_split*()
    @@ Commit message

         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.
    +    line. This removes the roundabout way of splitting the string into
    +    strbufs, just to return the line.

         Reported-by: Junio Hamano <gitster@pobox.com>
         Helped-by: Christian Couder <christian.couder@gmail.com>
    @@ gpg-interface.c: static char *get_default_ssh_signing_key(void)
      	int n;
      	char *default_key = NULL;
      	const char *literal_key = NULL;
    -+	char *begin, *new_line, *first_line, *end;
    ++	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)
     -		if (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) {
     +		begin = key_stdout.buf;
     +		new_line = strchr(begin, '\n');
    -+		end = new_line ? new_line : strchr(begin, '\0');
    -+		first_line = xmemdupz(begin, end - begin);
    ++		if (new_line)
    ++			first_line = xmemdupz(begin, new_line - begin);
    ++		else
    ++			first_line = xstrdup(begin);
     +		if (is_literal_ssh_key(first_line, &literal_key)) {
      			/*
      			 * We only use `is_literal_ssh_key` here to check validity

--
2.51.0.463.g79cf913ea9


  parent reply	other threads:[~2025-10-23 11:13 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-20 22:55 [Outreachy PATCH v4 0/2] do not use strbuf_split*() Olamide Caleb Bello
2025-10-20 22:55 ` [Outreachy PATCH v4 1/2] gpg-interface: do not use misdesigned strbuf_split*() Olamide Caleb Bello
2025-10-21  6:46   ` Christian Couder
2025-10-21  6:51     ` Christian Couder
2025-10-21 11:18     ` Bello Olamide
2025-10-21 16:26     ` Junio C Hamano
2025-10-20 22:55 ` [Outreachy PATCH v4 2/2] gpg-interface: do not use misdesigned strbuf_split*() [Part 2] Olamide Caleb Bello
2025-10-21  7:01   ` Christian Couder
2025-10-21 12:12     ` Bello Olamide
2025-10-21  7:19 ` [Outreachy PATCH v4 0/2] do not use strbuf_split*() Christian Couder
2025-10-21 10:19   ` Bello Olamide
2025-10-21 17:13     ` Junio C Hamano
2025-10-22  7:16       ` Bello Olamide
2025-10-22 12:40 ` [Outreachy PATCH v5 0/2] do not use misdesigned strbuf_split*() Olamide Caleb Bello
2025-10-22 12:40   ` [Outreachy PATCH v5 1/2] gpg-interface: " Olamide Caleb Bello
2025-10-22 13:56     ` Christian Couder
2025-10-23  8:14       ` Bello Olamide
2025-10-22 12:40   ` [Outreachy PATCH v5 2/2] " Olamide Caleb Bello
2025-10-22 14:03     ` Christian Couder
2025-10-22 17:44       ` Junio C Hamano
2025-10-23  8:17       ` Bello Olamide
2025-10-23 11:13   ` Olamide Caleb Bello [this message]
2025-10-23 11:13     ` [Outreachy PATCH v6 1/2] " Olamide Caleb Bello
2025-10-23 11:13     ` [Outreachy PATCH v6 2/2] " Olamide Caleb Bello
2025-10-23 16:27     ` [Outreachy PATCH v6 0/2] " Junio C Hamano
2025-10-24 13:25       ` Christian Couder

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.1761217100.git.belkid98@gmail.com \
    --to=belkid98@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kristofferhaugsbakk@fastmail.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).