Re: [PATCH 2/3] bundle.c: fix memory leak

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:04:07

Stefan Beller [off-list ref] writes:

On Tue, Mar 10, 2015 at 3:40 PM, Junio C Hamano [off-list ref] wrote:

quoted

Stefan Beller [off-list ref] writes:

quoted

The continue statements nearby also have an accompanying free(ref);

Signed-off-by: Stefan Beller <redacted>
---

I wonder what happens when dwim_ref() returned 2 or more, though.

That should also be fixed I guess. I'll look into it.

These one liner fixes are mostly done as a side project
having fun, just doing what the code analysis tools says,
sorry for not catching that one.

Maybe instead of the reoccuring pattern

    free(ref);
    continue;

we could just have a

    goto cleanup

which goes to the end of the loop where we have
the free anyway.

Yeah, I suspect that the end result may look more like that if you
cover the dwim_ref() one as well.

Thanks.

[PATCH 2/3] bundle.c: fix memory leak

From: Stefan Beller <hidden>
Date: 2016-06-15 23:04:07

There was one continue statement without an accompanying `free(ref)`.
Instead of adding that, replace all the free&&continue with a goto
just after writing the refs, where we'd do the free anyway and then
reloop.

Signed-off-by: Stefan Beller <redacted>
---
 bundle.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/bundle.c b/bundle.c
index 2e2dbd5..f732c92 100644
--- a/bundle.c
+++ b/bundle.c

@@ -334,7 +334,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 		if (e->item->flags & UNINTERESTING)
 			continue;
 		if (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)
-			continue;
+			goto skip_write_ref;
 		if (read_ref_full(e->name, RESOLVE_REF_READING, sha1, &flag))
 			flag = 0;
 		display_ref = (flag & REF_ISSYMREF) ? e->name : ref;

@@ -342,7 +342,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 		if (e->item->type == OBJ_TAG &&
 				!is_tag_in_date_range(e->item, revs)) {
 			e->item->flags |= UNINTERESTING;
-			continue;
+			goto skip_write_ref;
 		}
 
 		/*

@@ -357,8 +357,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 		if (!(e->item->flags & SHOWN) && e->item->type == OBJ_COMMIT) {
 			warning(_("ref '%s' is excluded by the rev-list options"),
 				e->name);
-			free(ref);
-			continue;
+			goto skip_write_ref;
 		}
 		/*
 		 * If you run "git bundle create bndl v1.0..v2.0", the

@@ -388,8 +387,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 				obj->flags |= SHOWN;
 				add_pending_object(revs, obj, e->name);
 			}
-			free(ref);
-			continue;
+			goto skip_write_ref;
 		}
 
 		ref_count++;

@@ -397,6 +395,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 		write_or_die(bundle_fd, " ", 1);
 		write_or_die(bundle_fd, display_ref, strlen(display_ref));
 		write_or_die(bundle_fd, "\n", 1);
+ skip_write_ref:
 		free(ref);
 	}

-- 
2.3.0.81.gc37f363

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help