[PATCH 2/3] format_config: simplify buffer handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxx>
---
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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]