{"thread":{"id":"10781","subject":"[PATCH] builtin-commit: fix --signoff","startedAt":"2007-11-10T05:49:14Z","lastAt":"2007-11-10T15:58:31Z","messageCount":5,"participants":["Johannes Schindelin","Junio C Hamano","Pierre Habouzit"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"59168","messageId":"Pine.LNX.4.64.0711100548071.4362@racer.site","threadId":"10781","inReplyTo":null,"subject":"[PATCH] builtin-commit: fix --signoff","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-10T05:49:14Z","receivedAt":"2007-11-10T05:49:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThe Signed-off-by: line contained a spurious timestamp.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin-commit.c |   31 ++++++++++++++++++++-----------\n 1 files changed, 20 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex e8bc4c4..f79ad48 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -181,21 +181,30 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \t\tdie(\"could not open %s\\n\", git_path(commit_editmsg));\n \n \tstripspace(&sb, 0);\n-\tif (fwrite(sb.buf, 1, sb.len, fp) < sb.len)\n-\t\tdie(\"could not write commit template: %s\\n\",\n-\t\t    strerror(errno));\n \n \tif (signoff) {\n-\t\tconst char *info, *bol;\n-\n-\t\tinfo = git_committer_info(1);\n-\t\tstrbuf_addch(&sb, '\\0');\n-\t\tbol = strrchr(sb.buf + sb.len - 1, '\\n');\n-\t\tif (!bol || prefixcmp(bol, sign_off_header))\n-\t\t\tfprintf(fp, \"\\n\");\n-\t\tfprintf(fp, \"%s%s\\n\", sign_off_header, git_committer_info(1));\n+\t\tstruct strbuf sob;\n+\t\tconst char *p;\n+\t\tint i;\n+\n+\t\tstrbuf_init(&sob, 0);\n+\t\tstrbuf_addstr(&sob, sign_off_header);\n+\t\tstrbuf_addstr(&sob, git_committer_info(1));\n+\t\tp = strrchr(sob.buf, '>');\n+\t\tif (p)\n+\t\t\tstrbuf_setlen(&sob, p + 1 - sob.buf);\n+\t\tstrbuf_addch(&sob, '\\n');\n+\n+\t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n+\t\t\t; /* do nothing */\n+\t\tif (prefixcmp(sb.buf + i, sob.buf))\n+\t\t\tstrbuf_addbuf(&sb, &sob);\n \t}\n \n+\tif (fwrite(sb.buf, 1, sb.len, fp) < sb.len)\n+\t\tdie(\"could not write commit template: %s\\n\",\n+\t\t    strerror(errno));\n+\n \tstrbuf_release(&sb);\n \n \tif (in_merge && !no_edit)\n-- \n1.5.3.5.1674.g6e7f7\n"},{"id":"59182","messageId":"7vr6iyo4ff.fsf@gitster.siamese.dyndns.org","threadId":"10781","inReplyTo":"Pine.LNX.4.64.0711100548071.4362@racer.site","subject":"Re: [PATCH] builtin-commit: fix --signoff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-10T09:06:28Z","receivedAt":"2007-11-10T09:06:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>  \tif (signoff) {\n> ...\n> +\t\tstrbuf_init(&sob, 0);\n> +\t\tstrbuf_addstr(&sob, sign_off_header);\n> +\t\tstrbuf_addstr(&sob, git_committer_info(1));\n> +\t\tp = strrchr(sob.buf, '>');\n> +\t\tif (p)\n> +\t\t\tstrbuf_setlen(&sob, p + 1 - sob.buf);\n> +\t\tstrbuf_addch(&sob, '\\n');\n> +\n> +\t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n> +\t\t\t; /* do nothing */\n> +\t\tif (prefixcmp(sb.buf + i, sob.buf))\n> +\t\t\tstrbuf_addbuf(&sb, &sob);\n>  \t}\n\nAt this point doesn't this leak sob.buf?\n"},{"id":"59209","messageId":"20071110131656.GD25204@artemis.corp","threadId":"10781","inReplyTo":"7vr6iyo4ff.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-commit: fix --signoff","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-11-10T13:16:56Z","receivedAt":"2007-11-10T13:16:56Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sat, Nov 10, 2007 at 09:06:28AM +0000, Junio C Hamano wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> >  \tif (signoff) {\n> > ...\n> > +\t\tstrbuf_init(&sob, 0);\n> > +\t\tstrbuf_addstr(&sob, sign_off_header);\n> > +\t\tstrbuf_addstr(&sob, git_committer_info(1));\n> > +\t\tp = strrchr(sob.buf, '>');\n> > +\t\tif (p)\n> > +\t\t\tstrbuf_setlen(&sob, p + 1 - sob.buf);\n> > +\t\tstrbuf_addch(&sob, '\\n');\n> > +\n> > +\t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n> > +\t\t\t; /* do nothing */\n> > +\t\tif (prefixcmp(sb.buf + i, sob.buf))\n> > +\t\t\tstrbuf_addbuf(&sb, &sob);\n> >  \t}\n> \n> At this point doesn't this leak sob.buf?\n\n  It does, strbuf_addbuf copies `sob` but doesn't release resources.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"59211","messageId":"Pine.LNX.4.64.0711101346120.4362@racer.site","threadId":"10781","inReplyTo":"7vr6iyo4ff.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-commit: fix --signoff","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-10T13:47:36Z","receivedAt":"2007-11-10T13:47:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 10 Nov 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> >  \tif (signoff) {\n> > ...\n> > +\t\tstrbuf_init(&sob, 0);\n> > +\t\tstrbuf_addstr(&sob, sign_off_header);\n> > +\t\tstrbuf_addstr(&sob, git_committer_info(1));\n> > +\t\tp = strrchr(sob.buf, '>');\n> > +\t\tif (p)\n> > +\t\t\tstrbuf_setlen(&sob, p + 1 - sob.buf);\n> > +\t\tstrbuf_addch(&sob, '\\n');\n> > +\n> > +\t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n> > +\t\t\t; /* do nothing */\n> > +\t\tif (prefixcmp(sb.buf + i, sob.buf))\n> > +\t\t\tstrbuf_addbuf(&sb, &sob);\n> >  \t}\n> \n> At this point doesn't this leak sob.buf?\n\nDarn.  I had a \"strbuf_release(&sob);\" there, but at some stage removed it \nby mistake.\n\nBut there is more to be fixed.  Rather than let git_committer_info() add \nthe timestamp, only to strip it away, is a waste.\n\nWill redo the patch.\n\nCiao,\nDscho\n"},{"id":"59218","messageId":"Pine.LNX.4.64.0711101557520.4362@racer.site","threadId":"10781","inReplyTo":"Pine.LNX.4.64.0711101346120.4362@racer.site","subject":"[PATCH] builtin-commit: fix --signoff","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-10T15:58:31Z","receivedAt":"2007-11-10T15:58:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThe Signed-off-by: line contained a spurious timestamp.  The reason was\na call to git_committer_info(1), which automatically added the\ntimestamp.\n\nInstead, fmt_ident() was taught to interpret an empty string for the\ndate (as opposed to NULL, which still triggers the default behavior)\nas \"do not bother with the timestamp\", and builtin-commit.c uses it.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tOn Sat, 10 Nov 2007, Johannes Schindelin wrote:\n\n\t> Will redo the patch.\n\n\tAnd here it is.\n\n builtin-commit.c  |   29 ++++++++++++++++++-----------\n ident.c           |   10 +++++++---\n t/t7500-commit.sh |   13 +++++++++++++\n 3 files changed, 38 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex e8bc4c4..a4d69ad 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -181,21 +181,28 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \t\tdie(\"could not open %s\\n\", git_path(commit_editmsg));\n \n \tstripspace(&sb, 0);\n-\tif (fwrite(sb.buf, 1, sb.len, fp) < sb.len)\n-\t\tdie(\"could not write commit template: %s\\n\",\n-\t\t    strerror(errno));\n \n \tif (signoff) {\n-\t\tconst char *info, *bol;\n-\n-\t\tinfo = git_committer_info(1);\n-\t\tstrbuf_addch(&sb, '\\0');\n-\t\tbol = strrchr(sb.buf + sb.len - 1, '\\n');\n-\t\tif (!bol || prefixcmp(bol, sign_off_header))\n-\t\t\tfprintf(fp, \"\\n\");\n-\t\tfprintf(fp, \"%s%s\\n\", sign_off_header, git_committer_info(1));\n+\t\tstruct strbuf sob;\n+\t\tint i;\n+\n+\t\tstrbuf_init(&sob, 0);\n+\t\tstrbuf_addstr(&sob, sign_off_header);\n+\t\tstrbuf_addstr(&sob, fmt_ident(getenv(\"GIT_COMMITTER_NAME\"),\n+                         getenv(\"GIT_COMMITTER_EMAIL\"), \"\", 1));\n+\t\tstrbuf_addch(&sob, '\\n');\n+\n+\t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n+\t\t\t; /* do nothing */\n+\t\tif (prefixcmp(sb.buf + i, sob.buf))\n+\t\t\tstrbuf_addbuf(&sb, &sob);\n+\t\tstrbuf_release(&sob);\n \t}\n \n+\tif (fwrite(sb.buf, 1, sb.len, fp) < sb.len)\n+\t\tdie(\"could not write commit template: %s\\n\",\n+\t\t    strerror(errno));\n+\n \tstrbuf_release(&sb);\n \n \tif (in_merge && !no_edit)\ndiff --git a/ident.c b/ident.c\nindex 9b2a852..5be7533 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -224,13 +224,17 @@ const char *fmt_ident(const char *name, const char *email,\n \t}\n \n \tstrcpy(date, git_default_date);\n-\tif (date_str)\n-\t\tparse_date(date_str, date, sizeof(date));\n+\tif (date_str) {\n+\t\tif (*date_str)\n+\t\t\tparse_date(date_str, date, sizeof(date));\n+\t\telse\n+\t\t\tdate[0] = '\\0';\n+\t}\n \n \ti = copy(buffer, sizeof(buffer), 0, name);\n \ti = add_raw(buffer, sizeof(buffer), i, \" <\");\n \ti = copy(buffer, sizeof(buffer), i, email);\n-\ti = add_raw(buffer, sizeof(buffer), i, \"> \");\n+\ti = add_raw(buffer, sizeof(buffer), i, date[0] ? \"> \" : \">\");\n \ti = copy(buffer, sizeof(buffer), i, date);\n \tif (i >= sizeof(buffer))\n \t\tdie(\"Impossibly long personal identifier\");\ndiff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\nindex abbf54b..13d5a0c 100755\n--- a/t/t7500-commit.sh\n+++ b/t/t7500-commit.sh\n@@ -93,4 +93,17 @@ test_expect_success 'commit message from file should override template' '\n \tcommit_msg_is \"standard input msg\"\n '\n \n+cat > expect << EOF\n+zort\n+Signed-off-by: C O Mitter <committer@example.com>\n+EOF\n+\n+test_expect_success '--signoff' '\n+\techo \"yet another content *narf*\" >> foo &&\n+\techo \"zort\" |\n+\t\tGIT_EDITOR=../t7500/add-content git commit -s -F - foo &&\n+\tgit cat-file commit HEAD | sed \"1,/^$/d\" > output &&\n+\tgit diff expect output\n+'\n+\n test_done\n-- \n1.5.3.5.1674.g6e7f7\n"}]}