Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
From: Cornelius Weig <hidden>
Date: 2017-01-31 13:18:47
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2017-01-31 · Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always · Junio C Hamano <hidden>
- 2017-01-31 · Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always · Cornelius Weig <hidden>
- 2017-01-31 · Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always · Jeff King <hidden>
- 2017-01-31 · Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always · Junio C Hamano <hidden>
- 2017-01-31 · Re: [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always · Junio C Hamano <hidden>
Hi,
The extra free(refname) is to plug the leak I pointed out, and the type of refname is no longer const, because "const char *" cannot be free()d without casting, and in this codepath I do not see a reason to mark it as const.
Ooops.. thanks for not yelling at me for that :-/
When queued on top of 4e59582ff7 ("Seventh batch for 2.12",
2017-01-23), however, this fails t2017#9 (orphan with -l makes
reflog when core.logAllRefUpdates = false).And again, thanks for not yelling. I overlooked that the "should_autocreate_reflog" return value should have been negated as shown below. Should I resend this patch, or is it easier for you to do the change yourself? Interdiff v2..v3:
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 81ea2ed..1e8631a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c@@ -612,8 +612,10 @@ static void update_refs_for_switch(const struct checkout_opts *opts, const char *old_desc, *reflog_msg; if (opts->new_branch) { if (opts->new_orphan_branch) { - const char *refname = mkpathdup("refs/heads/%s", opts->new_orphan_branch); - if (opts->new_branch_log && should_autocreate_reflog(refname)) { + char *refname; + + refname = mkpathdup("refs/heads/%s", opts->new_orphan_branch); + if (opts->new_branch_log && !should_autocreate_reflog(refname)) { int ret; struct strbuf err = STRBUF_INIT;
@@ -622,6 +624,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts, fprintf(stderr, _("Can not do reflog for '%s': %s\n"), opts->new_orphan_branch, err.buf); strbuf_release(&err); + free(refname); return; } strbuf_release(&err);