{"thread":{"id":"45009","subject":"[PATCHv4] builtin/commit.c: switch to strbuf, instead of snprintf()","startedAt":"2017-01-31T13:52:13Z","lastAt":"2017-01-31T13:52:13Z","messageCount":1,"participants":["Elia Pinto"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"310601","messageId":"20170131134535.35016-1-gitter.spiros@gmail.com","threadId":"45009","inReplyTo":null,"subject":"[PATCHv4] builtin/commit.c: switch to strbuf, instead of snprintf()","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2017-01-31T13:45:35Z","receivedAt":"2017-01-31T13:52:13Z","isPatch":false,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"Switch to dynamic allocation with strbuf, so we can avoid dealing\nwith magic numbers in the code and reduce the cognitive burden from\nthe programmers.  The original code is correct, but programmers no\nlonger have to count bytes needed for static allocation to know that.\n\nAs a side effect of this change, we also reduce the snprintf()\ncalls, that may silently truncate results if the programmer is not\ncareful.\n\nHelped-by: René Scharfe <l.s.r@web.de>\nHelped-by: Junio C Hamano <gitster@pobox.com>\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Elia Pinto <gitter.spiros@gmail.com>\n---\nThis is the fourth patch version.\n\nChanges from the first version: I have split the original commit in two, as discussed here\nhttp://public-inbox.org/git/20161213132717.42965-1-gitter.spiros@gmail.com/.\n\nChanges from the second version:\n- Changed the commit message to clarify the purpose of the patch (\nas suggested by Junio)\n\nChanges from the third version:\n- Swith to use strbuf instead of xstrfmt (\nas suggested by René Scharfe\nhere https://public-inbox.org/git/3165803b-6073-de97-bf33-71ad21108d6f@web.de/)\n\n\n builtin/commit.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 276c4f2e7..2de5f6cc6 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1525,12 +1525,10 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n static int run_rewrite_hook(const unsigned char *oldsha1,\n \t\t\t    const unsigned char *newsha1)\n {\n-\t/* oldsha1 SP newsha1 LF NUL */\n-\tstatic char buf[2*40 + 3];\n \tstruct child_process proc = CHILD_PROCESS_INIT;\n \tconst char *argv[3];\n \tint code;\n-\tsize_t n;\n+\tstruct strbuf sb = STRBUF_INIT;\n \n \targv[0] = find_hook(\"post-rewrite\");\n \tif (!argv[0])\n@@ -1546,11 +1544,11 @@ static int run_rewrite_hook(const unsigned char *oldsha1,\n \tcode = start_command(&proc);\n \tif (code)\n \t\treturn code;\n-\tn = snprintf(buf, sizeof(buf), \"%s %s\\n\",\n-\t\t     sha1_to_hex(oldsha1), sha1_to_hex(newsha1));\n+\tstrbuf_addf(&sb, \"%s %s\\n\", sha1_to_hex(oldsha1), sha1_to_hex(newsha1));\n \tsigchain_push(SIGPIPE, SIG_IGN);\n-\twrite_in_full(proc.in, buf, n);\n+\twrite_in_full(proc.in, sb.buf, sb.len);\n \tclose(proc.in);\n+\tstrbuf_release(&sb);\n \tsigchain_pop(SIGPIPE);\n \treturn finish_command(&proc);\n }\n-- \n2.11.0.297.g2a7e328.dirty\n\n"}]}