Download raw body.
plug a few small leaks in tog and repository.c:match_packed_object()
Combining them all inline as they are very small changes in just two
files: tog.c and repository.c (not including the last whitespace only
change in pack.c, spotted by chance while hunting down the
match_packed_object() leak).
Apart from log messages, I'll briefly elaborate on a couple as the diffs
themselves don't show the whole picture so it may save you having to
explore the code further, and I'm not certain on the original intent
behind the code producing the matched_ids leak, which we could fix a
couple ways (regress is happy with either). For this reason, I've opted
for the fix that maintains the current behaviour, but I want to draw
your attention to it just in case the status quo is not intended.
The id1 and id2 leak in the diff view (3rd commit) is because they are
first allocated in cmd_diff, but then we dup the ids immediately to
s->id1 and s->id2 in open_diff_view()--and only these copies are freed
in close_diff_view(). So we need to free the original allocations in
cmd_diff().
The matched_ids leak (5th commit) in repository.c:match_packed_object()
is because we initialise the potentially populated matched_ids list upon
entering pack.c:got_packidx_match_id_str_prefix(). We could zap that
STAILQ_INIT(matched_ids) call instead (final XX-prepended diff) but then
we would repeatedly loop over the same matched ids with each index file
as we'd be appending unique ids to the list from each index file, which
we are not presently doing because of that STAILQ_INIT(matched_ids) call
(although I'm not sure if that was the original intent?). So rather than
only free the list when the pack directory modification time changes,
free it after each index file iteration instead.
The log message of the remaining few commits should explain the fixes as
they're simple and self-explanatory but if not, please let me know!
-----------------------------------------------
commit b6fe41fdfb6219024a9afaea3da2065274a582ac
from: Mark Jamsek <mark@jamsek.dev>
date: Sat Dec 9 10:07:53 2023 UTC
tog: plug colors memleak in log view
M tog/tog.c | 3+ 6-
1 file changed, 3 insertions(+), 6 deletions(-)
diff 8fd0d1965f6393cf1ae1776daee76543055e0dd8 b6fe41fdfb6219024a9afaea3da2065274a582ac
commit - 8fd0d1965f6393cf1ae1776daee76543055e0dd8
commit + b6fe41fdfb6219024a9afaea3da2065274a582ac
blob - ce92dab6cd549c3c215b39c9561173732788a67c
blob + 42cbf71abb6e8b550dfb0a2331f340ca01f869de
--- tog/tog.c
+++ tog/tog.c
@@ -3535,6 +3535,7 @@ close_log_view(struct tog_view *view)
free_commits(&s->limit_commits);
free_commits(&s->real_commits);
+ free_colors(&s->colors);
free(s->in_repo_path);
s->in_repo_path = NULL;
free(s->start_id);
@@ -3845,16 +3846,12 @@ open_log_view(struct tog_view *view, struct got_object
goto done;
err = add_color(&s->colors, "^$", TOG_COLOR_AUTHOR,
get_color_value("TOG_COLOR_AUTHOR"));
- if (err) {
- free_colors(&s->colors);
+ if (err)
goto done;
- }
err = add_color(&s->colors, "^$", TOG_COLOR_DATE,
get_color_value("TOG_COLOR_DATE"));
- if (err) {
- free_colors(&s->colors);
+ if (err)
goto done;
- }
}
view->show = show_log_view;
-----------------------------------------------
commit 3c33690882bfa409292b75db9719c655b9402515
from: Mark Jamsek <mark@jamsek.dev>
date: Sat Dec 9 10:10:31 2023 UTC
tog: plug commit object leak in 'tog tree'
M tog/tog.c | 2+ 0-
1 file changed, 2 insertions(+), 0 deletions(-)
diff b6fe41fdfb6219024a9afaea3da2065274a582ac 3c33690882bfa409292b75db9719c655b9402515
commit - b6fe41fdfb6219024a9afaea3da2065274a582ac
commit + 3c33690882bfa409292b75db9719c655b9402515
blob - 42cbf71abb6e8b550dfb0a2331f340ca01f869de
blob + 1c4cd78b65b4cbc3e25d26ec8ee9aa66ec424bcd
--- tog/tog.c
+++ tog/tog.c
@@ -8355,6 +8355,8 @@ done:
free(cwd);
free(commit_id);
free(label);
+ if (commit != NULL)
+ got_object_commit_close(commit);
if (ref)
got_ref_close(ref);
if (worktree != NULL)
-----------------------------------------------
commit 5dd5fa1db32f0c250d6ad41baeebdcb4b2a5795d
from: Mark Jamsek <mark@jamsek.dev>
date: Sat Dec 9 10:15:03 2023 UTC
tog: plug object id leak in diff view
M tog/tog.c | 2+ 2-
1 file changed, 2 insertions(+), 2 deletions(-)
diff 3c33690882bfa409292b75db9719c655b9402515 5dd5fa1db32f0c250d6ad41baeebdcb4b2a5795d
commit - 3c33690882bfa409292b75db9719c655b9402515
commit + 5dd5fa1db32f0c250d6ad41baeebdcb4b2a5795d
blob - 1c4cd78b65b4cbc3e25d26ec8ee9aa66ec424bcd
blob + e2326392e5c40d180f4f231973f5e38f672ac161
--- tog/tog.c
+++ tog/tog.c
@@ -5606,8 +5606,6 @@ open_diff_view(struct tog_view *view, struct got_objec
s->last_displayed_line = view->nlines;
s->selected_line = 1;
s->repo = repo;
- s->id1 = id1;
- s->id2 = id2;
s->label1 = label1;
s->label2 = label2;
@@ -6196,6 +6194,8 @@ done:
free(keyword_idstr2);
free(label1);
free(label2);
+ free(id1);
+ free(id2);
free(repo_path);
free(cwd);
if (repo) {
-----------------------------------------------
commit 69e758a9c1e5024d68eac9094dc7691838af58db
from: Mark Jamsek <mark@jamsek.dev>
date: Sat Dec 9 11:42:11 2023 UTC
plug leak of commit object in 'tog diff' error path
Reduce the scope of the commit object too.
M tog/tog.c | 9+ 7-
1 file changed, 9 insertions(+), 7 deletions(-)
diff 5dd5fa1db32f0c250d6ad41baeebdcb4b2a5795d 69e758a9c1e5024d68eac9094dc7691838af58db
commit - 5dd5fa1db32f0c250d6ad41baeebdcb4b2a5795d
commit + 69e758a9c1e5024d68eac9094dc7691838af58db
blob - e2326392e5c40d180f4f231973f5e38f672ac161
blob + cde9c404b1d17c48cf1f941386d251fc795137d2
--- tog/tog.c
+++ tog/tog.c
@@ -5357,7 +5357,6 @@ create_diff(struct tog_diff_view_state *s)
case GOT_OBJ_TYPE_COMMIT: {
const struct got_object_id_queue *parent_ids;
struct got_object_qid *pid;
- struct got_commit_object *commit2;
struct got_reflist_head *refs;
size_t nlines = 0;
struct got_diffstat_cb_arg dsa = {
@@ -5382,9 +5381,6 @@ create_diff(struct tog_diff_view_state *s)
if (err)
break;
- err = got_object_open_as_commit(&commit2, s->repo, s->id2);
- if (err)
- goto done;
refs = got_reflist_object_id_map_lookup(tog_refs_idmap, s->id2);
/* Show commit info if we're diffing to a parent/root commit. */
if (s->id1 == NULL) {
@@ -5394,6 +5390,12 @@ create_diff(struct tog_diff_view_state *s)
if (err)
goto done;
} else {
+ struct got_commit_object *commit2;
+
+ err = got_object_open_as_commit(&commit2, s->repo,
+ s->id2);
+ if (err)
+ goto done;
parent_ids = got_object_commit_get_parent_ids(commit2);
STAILQ_FOREACH(pid, parent_ids, entry) {
if (got_object_id_cmp(s->id1, &pid->id) == 0) {
@@ -5401,13 +5403,13 @@ create_diff(struct tog_diff_view_state *s)
&s->nlines, s->id2, refs, s->repo,
s->ignore_whitespace,
s->force_text_diff, &dsa, s->f);
- if (err)
- goto done;
break;
}
}
+ got_object_commit_close(commit2);
+ if (err)
+ goto done;
}
- got_object_commit_close(commit2);
err = cat_diff(s->f, tmp_diff_file, &s->lines, &s->nlines,
lines, nlines);
-----------------------------------------------
commit 557f71d8d65ad007ecbe02338778f4911e49daa2
from: Mark Jamsek <mark@jamsek.dev>
date: Sat Dec 9 11:48:32 2023 UTC
plug object id queue leak when iterating pack index files
We need to free the matched object id queue on each pack index iteration--not
only when the pack file modification time has changed--otherwise the ids are
leaked when we reinitialise the queue in got_packidx_match_id_str_prefix().
M lib/repository.c | 2+ 1-
1 file changed, 2 insertions(+), 1 deletion(-)
diff 69e758a9c1e5024d68eac9094dc7691838af58db 557f71d8d65ad007ecbe02338778f4911e49daa2
commit - 69e758a9c1e5024d68eac9094dc7691838af58db
commit + 557f71d8d65ad007ecbe02338778f4911e49daa2
blob - a81092fd0836c5d97b58ad7ed9f24312f5c27691
blob + e66c6a1a0136362f4a6319872bc9f45cf2686704
--- lib/repository.c
+++ lib/repository.c
@@ -1809,7 +1809,6 @@ retry:
"modifications");
goto done;
}
- got_object_id_queue_free(&matched_ids);
goto retry;
}
@@ -1854,6 +1853,8 @@ retry:
goto done;
}
}
+ if (TAILQ_NEXT(pe, entry) != NULL)
+ got_object_id_queue_free(&matched_ids);
}
done:
got_object_id_queue_free(&matched_ids);
-----------------------------------------------
commit 828e5074bf59310d99fdc5be7e6c781ea4c8ba1a (main)
from: Mark Jamsek <mark@jamsek.dev>
date: Sat Dec 9 11:48:52 2023 UTC
whitespace
M lib/pack.c | 1+ 1-
1 file changed, 1 insertion(+), 1 deletion(-)
diff 557f71d8d65ad007ecbe02338778f4911e49daa2 828e5074bf59310d99fdc5be7e6c781ea4c8ba1a
commit - 557f71d8d65ad007ecbe02338778f4911e49daa2
commit + 828e5074bf59310d99fdc5be7e6c781ea4c8ba1a
blob - 09136af9ef6031bc1ba4935ddaba53108806ba54
blob + 76eb2dde64f751aa977e37a5be06c8601f1fc0e3
--- lib/pack.c
+++ lib/pack.c
@@ -695,7 +695,7 @@ got_packidx_match_id_str_prefix(struct got_object_id_q
int cmp;
if (!got_sha1_digest_to_str(oid->sha1, id_str, sizeof(id_str)))
- return got_error(GOT_ERR_NO_SPACE);
+ return got_error(GOT_ERR_NO_SPACE);
cmp = strncmp(id_str, id_str_prefix, prefix_len);
if (cmp < 0) {
--[[
XXX Alternative fix to the matched_ids leak that _changes_ the current
behaviour!
XXdiff e881d04f6380990aa7a8fca766418f8b09837cef ba875b8cf46ca0a63466ddb66b3d2cfb37c875ef
XXcommit - e881d04f6380990aa7a8fca766418f8b09837cef
XXcommit + ba875b8cf46ca0a63466ddb66b3d2cfb37c875ef
XXblob - 76eb2dde64f751aa977e37a5be06c8601f1fc0e3
XXblob + 762cf44cf20815499076425536178ffb4572414c
XX--- lib/pack.c
XX+++ lib/pack.c
XX@@ -675,8 +675,6 @@ got_packidx_match_id_str_prefix(struct got_object_id_q
XX struct got_packidx_object_id *oid;
XX uint32_t i = 0;
XX
XX- STAILQ_INIT(matched_ids);
XX-
XX if (prefix_len < 2)
XX return got_error_path(id_str_prefix, GOT_ERR_BAD_OBJ_ID_STR);
XX
--]]
--
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
plug a few small leaks in tog and repository.c:match_packed_object()