* [PATCH] builtin-commit: fix --signoff @ 2007-11-10 5:49 Johannes Schindelin 2007-11-10 9:06 ` Junio C Hamano 0 siblings, 1 reply; 5+ messages in thread From: Johannes Schindelin @ 2007-11-10 5:49 UTC (permalink / raw) To: git, krh, gitster The Signed-off-by: line contained a spurious timestamp. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- builtin-commit.c | 31 ++++++++++++++++++++----------- 1 files changed, 20 insertions(+), 11 deletions(-) diff --git a/builtin-commit.c b/builtin-commit.c index e8bc4c4..f79ad48 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -181,21 +181,30 @@ static int prepare_log_message(const char *index_file, const char *prefix) die("could not open %s\n", git_path(commit_editmsg)); stripspace(&sb, 0); - if (fwrite(sb.buf, 1, sb.len, fp) < sb.len) - die("could not write commit template: %s\n", - strerror(errno)); if (signoff) { - const char *info, *bol; - - info = git_committer_info(1); - strbuf_addch(&sb, '\0'); - bol = strrchr(sb.buf + sb.len - 1, '\n'); - if (!bol || prefixcmp(bol, sign_off_header)) - fprintf(fp, "\n"); - fprintf(fp, "%s%s\n", sign_off_header, git_committer_info(1)); + struct strbuf sob; + const char *p; + int i; + + strbuf_init(&sob, 0); + strbuf_addstr(&sob, sign_off_header); + strbuf_addstr(&sob, git_committer_info(1)); + p = strrchr(sob.buf, '>'); + if (p) + strbuf_setlen(&sob, p + 1 - sob.buf); + strbuf_addch(&sob, '\n'); + + for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--) + ; /* do nothing */ + if (prefixcmp(sb.buf + i, sob.buf)) + strbuf_addbuf(&sb, &sob); } + if (fwrite(sb.buf, 1, sb.len, fp) < sb.len) + die("could not write commit template: %s\n", + strerror(errno)); + strbuf_release(&sb); if (in_merge && !no_edit) -- 1.5.3.5.1674.g6e7f7 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] builtin-commit: fix --signoff 2007-11-10 5:49 [PATCH] builtin-commit: fix --signoff Johannes Schindelin @ 2007-11-10 9:06 ` Junio C Hamano 2007-11-10 13:16 ` Pierre Habouzit 2007-11-10 13:47 ` Johannes Schindelin 0 siblings, 2 replies; 5+ messages in thread From: Junio C Hamano @ 2007-11-10 9:06 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, krh, gitster Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > if (signoff) { > ... > + strbuf_init(&sob, 0); > + strbuf_addstr(&sob, sign_off_header); > + strbuf_addstr(&sob, git_committer_info(1)); > + p = strrchr(sob.buf, '>'); > + if (p) > + strbuf_setlen(&sob, p + 1 - sob.buf); > + strbuf_addch(&sob, '\n'); > + > + for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--) > + ; /* do nothing */ > + if (prefixcmp(sb.buf + i, sob.buf)) > + strbuf_addbuf(&sb, &sob); > } At this point doesn't this leak sob.buf? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] builtin-commit: fix --signoff 2007-11-10 9:06 ` Junio C Hamano @ 2007-11-10 13:16 ` Pierre Habouzit 2007-11-10 13:47 ` Johannes Schindelin 1 sibling, 0 replies; 5+ messages in thread From: Pierre Habouzit @ 2007-11-10 13:16 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin, git, krh [-- Attachment #1: Type: text/plain, Size: 907 bytes --] On Sat, Nov 10, 2007 at 09:06:28AM +0000, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > if (signoff) { > > ... > > + strbuf_init(&sob, 0); > > + strbuf_addstr(&sob, sign_off_header); > > + strbuf_addstr(&sob, git_committer_info(1)); > > + p = strrchr(sob.buf, '>'); > > + if (p) > > + strbuf_setlen(&sob, p + 1 - sob.buf); > > + strbuf_addch(&sob, '\n'); > > + > > + for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--) > > + ; /* do nothing */ > > + if (prefixcmp(sb.buf + i, sob.buf)) > > + strbuf_addbuf(&sb, &sob); > > } > > At this point doesn't this leak sob.buf? It does, strbuf_addbuf copies `sob` but doesn't release resources. -- ·O· Pierre Habouzit ··O madcoder@debian.org OOO http://www.madism.org [-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] builtin-commit: fix --signoff 2007-11-10 9:06 ` Junio C Hamano 2007-11-10 13:16 ` Pierre Habouzit @ 2007-11-10 13:47 ` Johannes Schindelin 2007-11-10 15:58 ` Johannes Schindelin 1 sibling, 1 reply; 5+ messages in thread From: Johannes Schindelin @ 2007-11-10 13:47 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, krh Hi, On Sat, 10 Nov 2007, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > if (signoff) { > > ... > > + strbuf_init(&sob, 0); > > + strbuf_addstr(&sob, sign_off_header); > > + strbuf_addstr(&sob, git_committer_info(1)); > > + p = strrchr(sob.buf, '>'); > > + if (p) > > + strbuf_setlen(&sob, p + 1 - sob.buf); > > + strbuf_addch(&sob, '\n'); > > + > > + for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--) > > + ; /* do nothing */ > > + if (prefixcmp(sb.buf + i, sob.buf)) > > + strbuf_addbuf(&sb, &sob); > > } > > At this point doesn't this leak sob.buf? Darn. I had a "strbuf_release(&sob);" there, but at some stage removed it by mistake. But there is more to be fixed. Rather than let git_committer_info() add the timestamp, only to strip it away, is a waste. Will redo the patch. Ciao, Dscho ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] builtin-commit: fix --signoff 2007-11-10 13:47 ` Johannes Schindelin @ 2007-11-10 15:58 ` Johannes Schindelin 0 siblings, 0 replies; 5+ messages in thread From: Johannes Schindelin @ 2007-11-10 15:58 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, krh The Signed-off-by: line contained a spurious timestamp. The reason was a call to git_committer_info(1), which automatically added the timestamp. Instead, fmt_ident() was taught to interpret an empty string for the date (as opposed to NULL, which still triggers the default behavior) as "do not bother with the timestamp", and builtin-commit.c uses it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- On Sat, 10 Nov 2007, Johannes Schindelin wrote: > Will redo the patch. And here it is. builtin-commit.c | 29 ++++++++++++++++++----------- ident.c | 10 +++++++--- t/t7500-commit.sh | 13 +++++++++++++ 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/builtin-commit.c b/builtin-commit.c index e8bc4c4..a4d69ad 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -181,21 +181,28 @@ static int prepare_log_message(const char *index_file, const char *prefix) die("could not open %s\n", git_path(commit_editmsg)); stripspace(&sb, 0); - if (fwrite(sb.buf, 1, sb.len, fp) < sb.len) - die("could not write commit template: %s\n", - strerror(errno)); if (signoff) { - const char *info, *bol; - - info = git_committer_info(1); - strbuf_addch(&sb, '\0'); - bol = strrchr(sb.buf + sb.len - 1, '\n'); - if (!bol || prefixcmp(bol, sign_off_header)) - fprintf(fp, "\n"); - fprintf(fp, "%s%s\n", sign_off_header, git_committer_info(1)); + struct strbuf sob; + int i; + + strbuf_init(&sob, 0); + strbuf_addstr(&sob, sign_off_header); + strbuf_addstr(&sob, fmt_ident(getenv("GIT_COMMITTER_NAME"), + getenv("GIT_COMMITTER_EMAIL"), "", 1)); + strbuf_addch(&sob, '\n'); + + for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--) + ; /* do nothing */ + if (prefixcmp(sb.buf + i, sob.buf)) + strbuf_addbuf(&sb, &sob); + strbuf_release(&sob); } + if (fwrite(sb.buf, 1, sb.len, fp) < sb.len) + die("could not write commit template: %s\n", + strerror(errno)); + strbuf_release(&sb); if (in_merge && !no_edit) diff --git a/ident.c b/ident.c index 9b2a852..5be7533 100644 --- a/ident.c +++ b/ident.c @@ -224,13 +224,17 @@ const char *fmt_ident(const char *name, const char *email, } strcpy(date, git_default_date); - if (date_str) - parse_date(date_str, date, sizeof(date)); + if (date_str) { + if (*date_str) + parse_date(date_str, date, sizeof(date)); + else + date[0] = '\0'; + } i = copy(buffer, sizeof(buffer), 0, name); i = add_raw(buffer, sizeof(buffer), i, " <"); i = copy(buffer, sizeof(buffer), i, email); - i = add_raw(buffer, sizeof(buffer), i, "> "); + i = add_raw(buffer, sizeof(buffer), i, date[0] ? "> " : ">"); i = copy(buffer, sizeof(buffer), i, date); if (i >= sizeof(buffer)) die("Impossibly long personal identifier"); diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh index abbf54b..13d5a0c 100755 --- a/t/t7500-commit.sh +++ b/t/t7500-commit.sh @@ -93,4 +93,17 @@ test_expect_success 'commit message from file should override template' ' commit_msg_is "standard input msg" ' +cat > expect << EOF +zort +Signed-off-by: C O Mitter <committer@example.com> +EOF + +test_expect_success '--signoff' ' + echo "yet another content *narf*" >> foo && + echo "zort" | + GIT_EDITOR=../t7500/add-content git commit -s -F - foo && + git cat-file commit HEAD | sed "1,/^$/d" > output && + git diff expect output +' + test_done -- 1.5.3.5.1674.g6e7f7 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-11-10 15:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-11-10 5:49 [PATCH] builtin-commit: fix --signoff Johannes Schindelin 2007-11-10 9:06 ` Junio C Hamano 2007-11-10 13:16 ` Pierre Habouzit 2007-11-10 13:47 ` Johannes Schindelin 2007-11-10 15:58 ` Johannes Schindelin
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 NNTP newsgroup(s).