From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "René Scharfe" <l.s.r@web.de>, git@vger.kernel.org
Subject: [PATCH v2 09/10] walker_fetch: fix minor memory leak
Date: Thu, 19 Jun 2014 17:29:48 -0400 [thread overview]
Message-ID: <20140619212948.GI28474@sigill.intra.peff.net> (raw)
In-Reply-To: <20140619211659.GA32412@sigill.intra.peff.net>
We sometimes allocate "msg" on the heap, but will fail to
free it if we hit the failure code path. We can instead keep
a separate variable that is safe to be freed no matter how
we get to the failure code path.
While we're here, we can also do two readability
improvements:
1. Use xstrfmt instead of a manual malloc/sprintf
2. Due to the "maybe we allocate msg, maybe we don't"
strategy, the logic for deciding which message to show
was split into two parts. Since the deallocation is now
pushed onto a separate variable, this is no longer a
concern, and we can keep all of the logic in the same
place.
Signed-off-by: Jeff King <peff@peff.net>
---
walker.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/walker.c b/walker.c
index 1dd86b8..0148264 100644
--- a/walker.c
+++ b/walker.c
@@ -253,7 +253,8 @@ int walker_fetch(struct walker *walker, int targets, char **target,
{
struct ref_lock **lock = xcalloc(targets, sizeof(struct ref_lock *));
unsigned char *sha1 = xmalloc(targets * 20);
- char *msg;
+ const char *msg;
+ char *to_free = NULL;
int ret;
int i;
@@ -285,21 +286,19 @@ int walker_fetch(struct walker *walker, int targets, char **target,
if (loop(walker))
goto unlock_and_fail;
- if (write_ref_log_details) {
- msg = xmalloc(strlen(write_ref_log_details) + 12);
- sprintf(msg, "fetch from %s", write_ref_log_details);
- } else {
- msg = NULL;
- }
+ if (write_ref_log_details)
+ msg = to_free = xstrfmt("fetch from %s", write_ref_log_details);
+ else
+ msg = "fetch (unknown)";
for (i = 0; i < targets; i++) {
if (!write_ref || !write_ref[i])
continue;
- ret = write_ref_sha1(lock[i], &sha1[20 * i], msg ? msg : "fetch (unknown)");
+ ret = write_ref_sha1(lock[i], &sha1[20 * i], msg);
lock[i] = NULL;
if (ret)
goto unlock_and_fail;
}
- free(msg);
+ free(to_free);
return 0;
@@ -307,6 +306,7 @@ unlock_and_fail:
for (i = 0; i < targets; i++)
if (lock[i])
unlock_ref(lock[i]);
+ free(to_free);
return -1;
}
--
2.0.0.566.gfe3e6b2
next prev parent reply other threads:[~2014-06-19 21:29 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-18 20:00 [PATCH 0/2] dropping manual malloc calculations Jeff King
2014-06-18 20:01 ` [PATCH 1/2] strbuf: add xstrdup_fmt helper Jeff King
2014-06-18 22:32 ` Junio C Hamano
2014-06-19 9:05 ` Jeff King
2014-06-19 16:49 ` Junio C Hamano
2014-06-19 21:16 ` [PATCH v2] dropping manual malloc calculations Jeff King
2014-06-19 21:18 ` [PATCH v2 01/10] strbuf: add xstrfmt helper Jeff King
2014-06-19 21:19 ` [PATCH v2 02/10] use xstrfmt in favor of manual size calculations Jeff King
2014-06-19 21:19 ` [PATCH v2 03/10] use xstrdup instead of xmalloc + strcpy Jeff King
2014-06-19 21:24 ` [PATCH v2 04/10] use xstrfmt to replace xmalloc + sprintf Jeff King
2014-06-19 21:26 ` [PATCH v2 05/10] use xstrfmt to replace xmalloc + strcpy/strcat Jeff King
2014-06-19 21:28 ` [PATCH v2 06/10] setup_git_env: use git_pathdup instead of xmalloc + sprintf Jeff King
2014-06-23 10:21 ` Eric Sunshine
2014-06-23 22:43 ` Junio C Hamano
2014-06-24 13:02 ` Duy Nguyen
2014-06-24 13:30 ` Duy Nguyen
2014-06-24 20:58 ` Jeff King
2014-06-25 12:37 ` Duy Nguyen
2014-06-25 17:20 ` Junio C Hamano
2014-06-25 17:22 ` Jeff King
2014-06-25 19:54 ` Junio C Hamano
2014-06-19 21:28 ` [PATCH v2 07/10] sequencer: use argv_array_pushf Jeff King
2014-06-19 21:29 ` [PATCH v2 08/10] merge: use argv_array when spawning merge strategy Jeff King
2014-06-19 21:29 ` Jeff King [this message]
2014-06-19 21:30 ` [PATCH v2 10/10] unique_path: fix unlikely heap overflow Jeff King
2014-06-19 16:52 ` [PATCH 1/2] strbuf: add xstrdup_fmt helper René Scharfe
2014-06-18 20:02 ` [PATCH 2/2] use xstrdup_fmt in favor of manual size calculations Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140619212948.GI28474@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=l.s.r@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).