walker_fetch: fix minor memory leak
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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Jun 19, 2014 at 17:29 UTC
f33206992de994424036a7d9912a968ab9829e6e
1 file changed
+9
-9
walker.c
+9
-9
@@ -253,7 +253,8 @@ int walker_fetch(struct walker *walker, int targets, char **target,
253
{
254
struct ref_lock **lock = xcalloc(targets, sizeof(struct ref_lock *));
255
unsigned char *sha1 = xmalloc(targets * 20);
256
- char *msg;
256
+ const char *msg;
257
+ char *to_free = NULL;
258
int ret;
259
int i;
260
@@ -285,21 +286,19 @@ int walker_fetch(struct walker *walker, int targets, char **target,
286
if (loop(walker))
287
goto unlock_and_fail;
288
288
- if (write_ref_log_details) {
289
- msg = xmalloc(strlen(write_ref_log_details) + 12);
290
- sprintf(msg, "fetch from %s", write_ref_log_details);
291
- } else {
292
- msg = NULL;
293
- }
289
+ if (write_ref_log_details)
290
+ msg = to_free = xstrfmt("fetch from %s", write_ref_log_details);
291
+ else
292
+ msg = "fetch (unknown)";
293
for (i = 0; i < targets; i++) {
294
if (!write_ref || !write_ref[i])
295
continue;
297
- ret = write_ref_sha1(lock[i], &sha1[20 * i], msg ? msg : "fetch (unknown)");
296
+ ret = write_ref_sha1(lock[i], &sha1[20 * i], msg);
297
lock[i] = NULL;
298
if (ret)
299
goto unlock_and_fail;
300
}
302
- free(msg);
301
+ free(to_free);
302
303
return 0;
304
@@ -307,6 +306,7 @@ unlock_and_fail:
306
for (i = 0; i < targets; i++)
307
if (lock[i])
308
unlock_ref(lock[i]);
309
+ free(to_free);
310
311
return -1;
312
}