All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Martin Ågren" <martin.agren@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>
Subject: [PATCH v2 4/6] object_array: use `object_array_clear()`, not `free()`
Date: Sat, 23 Sep 2017 01:34:52 +0200	[thread overview]
Message-ID: <f325af4048bc14d6194da169b02de7d18fff8471.1506120292.git.martin.agren@gmail.com> (raw)
In-Reply-To: <cover.1506120291.git.martin.agren@gmail.com>

Instead of freeing `foo.objects` for an object array `foo` (sometimes
conditionally), call `object_array_clear(&foo)`. This means we don't
poke as much into the implementation, which is already a good thing, but
also that we release the individual entries as well, thereby fixing at
least one memory-leak (in diff-lib.c).

If someone is holding on to a pointer to an element's `name` or `path`,
that is now a dangling pointer, i.e., we'd be turning an unpleasant
situation into an outright bug. To the best of my understanding no such
long-term pointers are being taken.

The way we handle `study` in builting/reflog.c still looks like it might
leak. That will be addressed in the next commit.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 builtin/reflog.c | 4 ++--
 diff-lib.c       | 3 +--
 submodule.c      | 4 ++--
 upload-pack.c    | 2 +-
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index e237d927a..6b34f23e7 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -182,8 +182,8 @@ static int commit_is_complete(struct commit *commit)
 			found.objects[i].item->flags |= SEEN;
 	}
 	/* free object arrays */
-	free(study.objects);
-	free(found.objects);
+	object_array_clear(&study);
+	object_array_clear(&found);
 	return !is_incomplete;
 }
 
diff --git a/diff-lib.c b/diff-lib.c
index 2a52b0795..4e0980caa 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -549,7 +549,6 @@ int index_differs_from(const char *def, int diff_flags,
 	rev.diffopt.flags |= diff_flags;
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
 	run_diff_index(&rev, 1);
-	if (rev.pending.alloc)
-		free(rev.pending.objects);
+	object_array_clear(&rev.pending);
 	return (DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0);
 }
diff --git a/submodule.c b/submodule.c
index 36f45f5a5..79fd01f7b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1728,7 +1728,7 @@ static int find_first_merges(struct object_array *result, const char *path,
 			add_object_array(merges.objects[i].item, NULL, result);
 	}
 
-	free(merges.objects);
+	object_array_clear(&merges);
 	return result->nr;
 }
 
@@ -1833,7 +1833,7 @@ int merge_submodule(struct object_id *result, const char *path,
 			print_commit((struct commit *) merges.objects[i].item);
 	}
 
-	free(merges.objects);
+	object_array_clear(&merges);
 	return 0;
 }
 
diff --git a/upload-pack.c b/upload-pack.c
index 7efff2fbf..ec0eee8fc 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -888,7 +888,7 @@ static void receive_needs(void)
 		}
 
 	shallow_nr += shallows.nr;
-	free(shallows.objects);
+	object_array_clear(&shallows);
 }
 
 /* return non-zero if the ref is hidden, otherwise 0 */
-- 
2.14.1.727.g9ddaf86


  parent reply	other threads:[~2017-09-22 23:35 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-20 19:47 [PATCH] diff-lib: clear `pending` object-array in `index_differs_from()` Martin Ågren
2017-09-20 20:02 ` Jeff King
2017-09-21  3:56   ` Martin Ågren
2017-09-21  4:52     ` Jeff King
2017-09-22 23:34   ` [PATCH v2 0/6] reroll ma/plugleaks; more `object_array`-fixes Martin Ågren
2017-09-22 23:34     ` [PATCH v2 1/6] builtin/commit: fix memory leak in `prepare_index()` Martin Ågren
2017-09-22 23:34     ` [PATCH v2 2/6] commit: fix memory leak in `reduce_heads()` Martin Ågren
2017-09-22 23:34     ` [PATCH v2 3/6] leak_pending: use `object_array_clear()`, not `free()` Martin Ågren
2017-09-23  3:47       ` Jeff King
2017-09-22 23:34     ` Martin Ågren [this message]
2017-09-23  4:04       ` [PATCH v2 4/6] object_array: " Jeff King
2017-09-23  9:41         ` Martin Ågren
2017-09-22 23:34     ` [PATCH v2 5/6] object_array: add and use `object_array_pop()` Martin Ågren
2017-09-23  4:27       ` Jeff King
2017-09-23  9:49         ` Martin Ågren
2017-09-23 15:47           ` Jeff King
2017-09-22 23:34     ` [PATCH v2 6/6] pack-bitmap[-write]: use `object_array_clear()`, don't leak Martin Ågren
2017-09-23  4:35       ` Jeff King
2017-09-23  4:37     ` [PATCH v2 0/6] reroll ma/plugleaks; more `object_array`-fixes Jeff King
2017-09-23  9:54       ` Martin Ågren
2017-09-23 16:13         ` Jeff King
2017-09-23 16:38           ` Jeff King
2017-09-24 19:59             ` Martin Ågren
2017-09-25 16:08               ` Jeff King
2017-10-01 15:04                 ` Martin Ågren
2017-09-24  7:01     ` Junio C Hamano
2017-09-24 20:00       ` Martin Ågren

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=f325af4048bc14d6194da169b02de7d18fff8471.1506120292.git.martin.agren@gmail.com \
    --to=martin.agren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.