[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    [PATCHv4] builtin/commit.c: switch to strbuf, instead of snprintf()
From:       Elia Pinto <gitter.spiros () gmail ! com>
Date:       2017-01-31 13:45:35
Message-ID: 20170131134535.35016-1-gitter.spiros () gmail ! com
[Download RAW message or body]

Switch to dynamic allocation with strbuf, so we can avoid dealing
with magic numbers in the code and reduce the cognitive burden from
the programmers.  The original code is correct, but programmers no
longer have to count bytes needed for static allocation to know that.

As a side effect of this change, we also reduce the snprintf()
calls, that may silently truncate results if the programmer is not
careful.

Helped-by: René Scharfe <l.s.r@web.de>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
This is the fourth patch version.

Changes from the first version: I have split the original commit in two, as discussed here
http://public-inbox.org/git/20161213132717.42965-1-gitter.spiros@gmail.com/.

Changes from the second version:
- Changed the commit message to clarify the purpose of the patch (
as suggested by Junio)

Changes from the third version:
- Swith to use strbuf instead of xstrfmt (
as suggested by René Scharfe
here https://public-inbox.org/git/3165803b-6073-de97-bf33-71ad21108d6f@web.de/)


 builtin/commit.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 276c4f2e7..2de5f6cc6 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1525,12 +1525,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 static int run_rewrite_hook(const unsigned char *oldsha1,
 			    const unsigned char *newsha1)
 {
-	/* oldsha1 SP newsha1 LF NUL */
-	static char buf[2*40 + 3];
 	struct child_process proc = CHILD_PROCESS_INIT;
 	const char *argv[3];
 	int code;
-	size_t n;
+	struct strbuf sb = STRBUF_INIT;
 
 	argv[0] = find_hook("post-rewrite");
 	if (!argv[0])
@@ -1546,11 +1544,11 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
 	code = start_command(&proc);
 	if (code)
 		return code;
-	n = snprintf(buf, sizeof(buf), "%s %s\n",
-		     sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
+	strbuf_addf(&sb, "%s %s\n", sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
 	sigchain_push(SIGPIPE, SIG_IGN);
-	write_in_full(proc.in, buf, n);
+	write_in_full(proc.in, sb.buf, sb.len);
 	close(proc.in);
+	strbuf_release(&sb);
 	sigchain_pop(SIGPIPE);
 	return finish_command(&proc);
 }
-- 
2.11.0.297.g2a7e328.dirty

[prev in list] [next in list] [prev in thread] [next in thread]