From f131db9e3166c528d3b0352b653eb0d9deca5a65 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 26 Apr 2017 23:25:55 -0400 Subject: am: fix commit buffer leak in get_commit_info() Calling logmsg_reencode() may allocate a buffer for the commit message (because we need to load it from disk, or because it needs re-encoded). We must "unuse" it afterwards to free it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/am.c | 1 + 1 file changed, 1 insertion(+) (limited to 'builtin') diff --git a/builtin/am.c b/builtin/am.c index 826f18ba12..8dfe8f84fa 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1417,6 +1417,7 @@ static void get_commit_info(struct am_state *state, struct commit *commit) die(_("unable to parse commit %s"), oid_to_hex(&commit->object.oid)); state->msg = xstrdup(msg + 2); state->msg_len = strlen(state->msg); + unuse_commit_buffer(commit, buffer); } /** -- cgit v1.2.3 From 2e2bbb9624e10537560c3ac45ff6820ff773b3d6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 26 Apr 2017 23:27:17 -0400 Subject: am: simplify allocations in get_commit_info() After we call split_ident_line(), we have several begin/end pairs for various parts of the ident. We then copy each into a strbuf to create a single string, and then detach that string. We can instead skip the strbuf entirely and just duplicate the strings directly. This is shorter, and it makes it more obvious that we are not leaking the strbuf (we were not before, because every code path either died or hit a strbuf_detach). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/am.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) (limited to 'builtin') diff --git a/builtin/am.c b/builtin/am.c index 8dfe8f84fa..17f394c825 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1376,40 +1376,35 @@ static int get_mail_commit_oid(struct object_id *commit_id, const char *mail) */ static void get_commit_info(struct am_state *state, struct commit *commit) { - const char *buffer, *ident_line, *author_date, *msg; + const char *buffer, *ident_line, *msg; size_t ident_len; struct ident_split ident_split; - struct strbuf sb = STRBUF_INIT; buffer = logmsg_reencode(commit, NULL, get_commit_output_encoding()); ident_line = find_commit_header(buffer, "author", &ident_len); - if (split_ident_line(&ident_split, ident_line, ident_len) < 0) { - strbuf_add(&sb, ident_line, ident_len); - die(_("invalid ident line: %s"), sb.buf); - } + if (split_ident_line(&ident_split, ident_line, ident_len) < 0) + die(_("invalid ident line: %.*s"), (int)ident_len, ident_line); assert(!state->author_name); if (ident_split.name_begin) { - strbuf_add(&sb, ident_split.name_begin, - ident_split.name_end - ident_split.name_begin); - state->author_name = strbuf_detach(&sb, NULL); + state->author_name = + xmemdupz(ident_split.name_begin, + ident_split.name_end - ident_split.name_begin); } else state->author_name = xstrdup(""); assert(!state->author_email); if (ident_split.mail_begin) { - strbuf_add(&sb, ident_split.mail_begin, - ident_split.mail_end - ident_split.mail_begin); - state->author_email = strbuf_detach(&sb, NULL); + state->author_email = + xmemdupz(ident_split.mail_begin, + ident_split.mail_end - ident_split.mail_begin); } else state->author_email = xstrdup(""); - author_date = show_ident_date(&ident_split, DATE_MODE(NORMAL)); - strbuf_addstr(&sb, author_date); assert(!state->author_date); - state->author_date = strbuf_detach(&sb, NULL); + state->author_date = xstrdup(show_ident_date(&ident_split, DATE_MODE(NORMAL))); assert(!state->msg); msg = strstr(buffer, "\n\n"); -- cgit v1.2.3 From 721f5f1e355648f6b15d79799a3fdc6b4a28352f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 26 Apr 2017 23:28:31 -0400 Subject: am: shorten ident_split variable name in get_commit_info() The local ident_split variable is often mentioned three times per line when dealing with its begin/end pointer pairs. Let's use a shorter name which lets us get rid of some long lines. Since this is a short self-contained function, readability doesn't suffer. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/am.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'builtin') diff --git a/builtin/am.c b/builtin/am.c index 17f394c825..5c747faa05 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1378,33 +1378,31 @@ static void get_commit_info(struct am_state *state, struct commit *commit) { const char *buffer, *ident_line, *msg; size_t ident_len; - struct ident_split ident_split; + struct ident_split id; buffer = logmsg_reencode(commit, NULL, get_commit_output_encoding()); ident_line = find_commit_header(buffer, "author", &ident_len); - if (split_ident_line(&ident_split, ident_line, ident_len) < 0) + if (split_ident_line(&id, ident_line, ident_len) < 0) die(_("invalid ident line: %.*s"), (int)ident_len, ident_line); assert(!state->author_name); - if (ident_split.name_begin) { + if (id.name_begin) state->author_name = - xmemdupz(ident_split.name_begin, - ident_split.name_end - ident_split.name_begin); - } else + xmemdupz(id.name_begin, id.name_end - id.name_begin); + else state->author_name = xstrdup(""); assert(!state->author_email); - if (ident_split.mail_begin) { + if (id.mail_begin) state->author_email = - xmemdupz(ident_split.mail_begin, - ident_split.mail_end - ident_split.mail_begin); - } else + xmemdupz(id.mail_begin, id.mail_end - id.mail_begin); + else state->author_email = xstrdup(""); assert(!state->author_date); - state->author_date = xstrdup(show_ident_date(&ident_split, DATE_MODE(NORMAL))); + state->author_date = xstrdup(show_ident_date(&id, DATE_MODE(NORMAL))); assert(!state->msg); msg = strstr(buffer, "\n\n"); -- cgit v1.2.3