git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "SZEDER Gábor" <szeder@ira.uka.de>
Cc: Junio C Hamano <gitster@pobox.com>,
	Christian Couder <christian.couder@gmail.com>,
	git@vger.kernel.org
Subject: [PATCH 2/3] format_config: simplify buffer handling
Date: Thu, 20 Aug 2015 10:47:34 -0400	[thread overview]
Message-ID: <20150820144733.GB11913@sigill.intra.peff.net> (raw)
In-Reply-To: <20150820144504.GA22935@sigill.intra.peff.net>

When formatting a config value into a strbuf, we may end
up stringifying it into a fixed-size buffer using sprintf,
and then copying that buffer into the strbuf. We can
eliminate the middle-man (and drop some calls to sprintf!)
by writing directly to the strbuf.

The reason it was written this way in the first place is
that we need to know before writing the value whether to
insert a delimiter. Instead of delaying the write of the
value, we speculatively write the delimiter, and roll it
back in the single case that cares.

Signed-off-by: Jeff King <peff@peff.net>
---
I admit the rollback is a little gross. The other option would be adding
the delimiter in each of the conditional branches, which is also kind of
nasty. Or we could leave the buffer-write as-is, and replace the
sprintfs with snprintfs (this is actually something I was doing in
another branch, which is why I took particular notice; your patch
conflicts with my to-be-submitted branch :) ).

 builtin/config.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 91aa56f..04befce 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -111,41 +111,35 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value
 	if (show_keys)
 		strbuf_addstr(buf, key_);
 	if (!omit_values) {
-		int must_free_vptr = 0;
-		int must_add_delim = show_keys;
-		char value[256];
-		const char *vptr = value;
+		if (show_keys)
+			strbuf_addch(buf, key_delim);
 
 		if (types == TYPE_INT)
-			sprintf(value, "%"PRId64,
-				git_config_int64(key_, value_ ? value_ : ""));
+			strbuf_addf(buf, "%"PRId64,
+				    git_config_int64(key_, value_ ? value_ : ""));
 		else if (types == TYPE_BOOL)
-			vptr = git_config_bool(key_, value_) ? "true" : "false";
+			strbuf_addstr(buf, git_config_bool(key_, value_) ?
+				      "true" : "false");
 		else if (types == TYPE_BOOL_OR_INT) {
 			int is_bool, v;
 			v = git_config_bool_or_int(key_, value_, &is_bool);
 			if (is_bool)
-				vptr = v ? "true" : "false";
+				strbuf_addstr(buf, v ? "true" : "false");
 			else
-				sprintf(value, "%d", v);
+				strbuf_addf(buf, "%d", v);
 		} else if (types == TYPE_PATH) {
-			if (git_config_pathname(&vptr, key_, value_) < 0)
+			const char *v;
+			if (git_config_pathname(&v, key_, value_) < 0)
 				return -1;
-			must_free_vptr = 1;
+			strbuf_addstr(buf, v);
+			free((char *)v);
 		} else if (value_) {
-			vptr = value_;
+			strbuf_addstr(buf, value_);
 		} else {
-			/* Just show the key name */
-			vptr = "";
-			must_add_delim = 0;
+			/* Just show the key name; back out delimiter */
+			if (show_keys)
+				strbuf_setlen(buf, buf->len - 1);
 		}
-
-		if (must_add_delim)
-			strbuf_addch(buf, key_delim);
-		strbuf_addstr(buf, vptr);
-
-		if (must_free_vptr)
-			free((char *)vptr);
 	}
 	strbuf_addch(buf, term);
 	return 0;
-- 
2.5.0.680.g69e7703

  parent reply	other threads:[~2015-08-20 14:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-10  9:46 [PATCHv3 0/2] 'git config --names-only' to help the completion script SZEDER Gábor
2015-08-10  9:46 ` [PATCHv3 1/2] config: add '--names-only' option to list only variable names SZEDER Gábor
2015-08-10 13:41   ` Jeff King
2015-08-10 17:18   ` Junio C Hamano
2015-08-12 23:47     ` SZEDER Gábor
2015-08-13  1:39       ` Junio C Hamano
2015-08-20 14:14         ` [PATCH] config: restructure format_config() for better control flow SZEDER Gábor
2015-08-20 14:45           ` Jeff King
2015-08-20 14:46             ` [PATCH 1/3] format_config: don't init strbuf Jeff King
2015-08-20 14:47             ` Jeff King [this message]
2015-08-21 11:52               ` [PATCH 2/3] format_config: simplify buffer handling SZEDER Gábor
2015-08-21 17:42               ` Junio C Hamano
2015-08-21 17:43                 ` Junio C Hamano
2015-08-21 19:40                   ` Jeff King
2015-08-20 14:49             ` [PATCH 3/3] get_urlmatch: avoid useless strbuf write Jeff King
2015-08-20 20:19             ` [PATCH] config: restructure format_config() for better control flow Junio C Hamano
2015-08-10  9:46 ` [PATCHv3 2/2] completion: list variable names reliably with 'git config --names-only' SZEDER Gábor
2015-08-10 13:45   ` Jeff King

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150820144733.GB11913@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=szeder@ira.uka.de \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).