summaryrefslogtreecommitdiff
path: root/builtin/reflog.c
diff options
context:
space:
mode:
authorMartin Ågren <martin.agren@gmail.com>2017-09-22 23:34:53 (GMT)
committerJunio C Hamano <gitster@pobox.com>2017-09-24 01:06:04 (GMT)
commit719920393737b3934a168f35ab45e09104edeed8 (patch)
tree5e68a3dfd103a03036d371b1eb3f0858f4f51d8d /builtin/reflog.c
parentdcb572ab94f83a1a857d276fcebff5700077f2b7 (diff)
downloadgit-719920393737b3934a168f35ab45e09104edeed8.zip
git-719920393737b3934a168f35ab45e09104edeed8.tar.gz
git-719920393737b3934a168f35ab45e09104edeed8.tar.bz2
object_array: add and use `object_array_pop()`
In a couple of places, we pop objects off an object array `foo` by decreasing `foo.nr`. We access `foo.nr` in many places, but most if not all other times we do so read-only, e.g., as we iterate over the array. But when we change `foo.nr` behind the array's back, it feels a bit nasty and looks like it might leak memory. Leaks happen if the popped element has an allocated `name` or `path`. At the moment, that is not the case. Still, 1) the object array might gain more fields that want to be freed, 2) a code path where we pop might start using names or paths, 3) one of these code paths might be copied to somewhere where we do, and 4) using a dedicated function for popping is conceptually cleaner. Introduce and use `object_array_pop()` instead. Release memory in the new function. Document that popping an object leaves the associated elements in limbo. The converted places were identified by grepping for "\.nr\>" and looking for "--". Make the new function return NULL on an empty array. This is consistent with `pop_commit()` and allows the following: while ((o = object_array_pop(&foo)) != NULL) { // do something } But as noted above, we don't need to go out of our way to avoid reading `foo.nr`. This is probably more readable: while (foo.nr) { ... o = object_array_pop(&foo); // do something } The name of `object_array_pop()` does not quite align with `add_object_array()`. That is unfortunate. On the other hand, it matches `object_array_clear()`. Arguably it's `add_...` that is the odd one out, since it reads like it's used to "add" an "object array". For that reason, side with `object_array_clear()`. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'builtin/reflog.c')
-rw-r--r--builtin/reflog.c2
1 files changed, 1 insertions, 1 deletions
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 6b34f23..2067cca 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -126,7 +126,7 @@ static int commit_is_complete(struct commit *commit)
struct commit *c;
struct commit_list *parent;
- c = (struct commit *)study.objects[--study.nr].item;
+ c = (struct commit *)object_array_pop(&study);
if (!c->object.parsed && !parse_object(&c->object.oid))
c->object.flags |= INCOMPLETE;