Download raw body.
got: cleanup failure leaks and set error on asprintf() failure
As per the subject, the following two diffs perform some minor
housekeeping.
First are some failure leaks spotted while implementing diffstat in
lib/diff.c (some of which were also observed by op whose idea I went
with instead of the original fix to free at every error return).
The second is catching a couple instances I missed in got.c:cmd_diff()
where error isn't set on asprintf() failure even though we goto done.
ok?
-----------------------------------------------
commit 43adae771e15759422dd9d9f229be7c77ffe4579 (main)
from: Mark Jamsek <mark@jamsek.dev>
date: Wed Jan 11 10:44:49 2023 UTC
got: style(9) and cleanup failure leaks lib/diff.c
Spotted while implementing diffstat plus one introduced with diffstat code.
Don't leak 'change' on got_pathlist_append() error. And don't leak modestr{1,2}
and l{1,2} char pointers in diff_blobs() and got_diff_tree(), respecitvely.
Regarding modestr leaks, rather than free at all error return points, use op's
suggestion to lift modestr vars to function scope.
M lib/diff.c | 42+ 20-
1 file changed, 42 insertions(+), 20 deletions(-)
diff 47504e80cba47264929a34e7311b2a98cea85b61 43adae771e15759422dd9d9f229be7c77ffe4579
commit - 47504e80cba47264929a34e7311b2a98cea85b61
commit + 43adae771e15759422dd9d9f229be7c77ffe4579
blob - 9ada3529102e62be359b43263e1847b0fc5a7ccc
blob + 6f9c397cb5beed5d6de4ed2b82014650732b3318
--- lib/diff.c
+++ lib/diff.c
@@ -114,8 +114,10 @@ get_diffstat(struct got_diffstat_cb_arg *ds, const cha
++ds->nfiles;
err = got_pathlist_append(ds->paths, path, change);
- if (err)
+ if (err) {
+ free(change);
return err;
+ }
pe = TAILQ_LAST(ds->paths, got_pathlist_head);
diffstat_field_width(&ds->max_path_len, &ds->add_cols, &ds->rm_cols,
@@ -137,6 +139,7 @@ diff_blobs(struct got_diff_line **lines, size_t *nline
char hex1[SHA1_DIGEST_STRING_LENGTH];
char hex2[SHA1_DIGEST_STRING_LENGTH];
const char *idstr1 = NULL, *idstr2 = NULL;
+ char *modestr1 = NULL, *modestr2 = NULL;
off_t size1, size2;
struct got_diffreg_result *result = NULL;
off_t outoff = 0;
@@ -185,8 +188,8 @@ diff_blobs(struct got_diff_line **lines, size_t *nline
idstr2 = "/dev/null";
if (outfile) {
- char *modestr1 = NULL, *modestr2 = NULL;
int modebits;
+
if (mode1 && mode1 != mode2) {
if (S_ISLNK(mode1))
modebits = S_IFLNK;
@@ -232,9 +235,6 @@ diff_blobs(struct got_diff_line **lines, size_t *nline
if (err)
goto done;
}
-
- free(modestr1);
- free(modestr2);
}
err = got_diffreg(&result, f1, f2, diff_algo, ignore_whitespace,
@@ -296,6 +296,8 @@ done:
}
done:
+ free(modestr1);
+ free(modestr2);
if (resultp && err == NULL)
*resultp = result;
else if (result) {
@@ -896,13 +898,16 @@ got_diff_tree(struct got_tree_object *tree1, struct go
if (tree2) {
te2 = got_object_tree_get_entry(tree2, 0);
if (te2 && asprintf(&l2, "%s%s%s", label2, label2[0] ? "/" : "",
- te2->name) == -1)
- return got_error_from_errno("asprintf");
+ te2->name) == -1) {
+ err = got_error_from_errno("asprintf");
+ goto done;
+ }
}
do {
if (te1) {
struct got_tree_entry *te = NULL;
+
if (tree2)
te = got_object_tree_find_entry(tree2,
te1->name);
@@ -910,10 +915,12 @@ got_diff_tree(struct got_tree_object *tree1, struct go
free(l2);
l2 = NULL;
if (te && asprintf(&l2, "%s%s%s", label2,
- label2[0] ? "/" : "", te->name) == -1)
- return
- got_error_from_errno("asprintf");
+ label2[0] ? "/" : "", te->name) == -1) {
+ err = got_error_from_errno("asprintf");
+ goto done;
+ }
}
+
err = diff_entry_old_new(te1, te, f1, f2, fd1, fd2,
l1, l2, repo, cb, cb_arg, diff_content);
if (err)
@@ -922,21 +929,27 @@ got_diff_tree(struct got_tree_object *tree1, struct go
if (te2) {
struct got_tree_entry *te = NULL;
+
if (tree1)
te = got_object_tree_find_entry(tree1,
te2->name);
+
free(l2);
+ l2 = NULL;
if (te) {
if (asprintf(&l2, "%s%s%s", label2,
- label2[0] ? "/" : "", te->name) == -1)
- return
- got_error_from_errno("asprintf");
+ label2[0] ? "/" : "", te->name) == -1) {
+ err = got_error_from_errno("asprintf");
+ goto done;
+ }
} else {
if (asprintf(&l2, "%s%s%s", label2,
- label2[0] ? "/" : "", te2->name) == -1)
- return
- got_error_from_errno("asprintf");
+ label2[0] ? "/" : "", te2->name) == -1) {
+ err = got_error_from_errno("asprintf");
+ goto done;
+ }
}
+
err = diff_entry_new_old(te2, te, f1, f2, fd2, l2,
repo, cb, cb_arg, diff_content);
if (err)
@@ -950,9 +963,12 @@ got_diff_tree(struct got_tree_object *tree1, struct go
te1 = got_object_tree_get_entry(tree1, tidx1);
if (te1 &&
asprintf(&l1, "%s%s%s", label1,
- label1[0] ? "/" : "", te1->name) == -1)
- return got_error_from_errno("asprintf");
+ label1[0] ? "/" : "", te1->name) == -1) {
+ err = got_error_from_errno("asprintf");
+ goto done;
+ }
}
+
free(l2);
l2 = NULL;
if (te2) {
@@ -960,11 +976,16 @@ got_diff_tree(struct got_tree_object *tree1, struct go
te2 = got_object_tree_get_entry(tree2, tidx2);
if (te2 &&
asprintf(&l2, "%s%s%s", label2,
- label2[0] ? "/" : "", te2->name) == -1)
- return got_error_from_errno("asprintf");
+ label2[0] ? "/" : "", te2->name) == -1) {
+ err = got_error_from_errno("asprintf");
+ goto done;
+ }
}
} while (te1 || te2);
+done:
+ free(l1);
+ free(l2);
return err;
}
@@ -1377,6 +1398,7 @@ done:
*resultp = diffreg_result;
else if (diffreg_result) {
const struct got_error *free_err;
+
free_err = got_diffreg_result_free(diffreg_result);
if (free_err && err == NULL)
err = free_err;
-----------------------------------------------
commit 47504e80cba47264929a34e7311b2a98cea85b61
from: Mark Jamsek <mark@jamsek.dev>
date: Wed Jan 11 10:44:48 2023 UTC
got: set error on asprintf() failure
M got/got.c | 6+ 2-
1 file changed, 6 insertions(+), 2 deletions(-)
diff 00b3e9ae14f04a45f1ca7445bade6b41a6e8a1c5 47504e80cba47264929a34e7311b2a98cea85b61
commit - 00b3e9ae14f04a45f1ca7445bade6b41a6e8a1c5
commit + 47504e80cba47264929a34e7311b2a98cea85b61
blob - 850056f73d0f8671f6fc63162e2753e92b8b8c41
blob + a20fdf567d2fb43a228a54708fee12c13f94796b
--- got/got.c
+++ got/got.c
@@ -5249,8 +5249,10 @@ cmd_diff(int argc, char *argv[])
if (asprintf(&header, "diffstat %s%s",
diff_staged ? "-s " : "",
- got_worktree_get_root_path(worktree)) == -1)
+ got_worktree_get_root_path(worktree)) == -1) {
+ error = got_error_from_errno("asprintf");
goto done;
+ }
error = print_diffstat(&dsa, &diffstat_paths, header);
free(header);
@@ -5421,8 +5423,10 @@ cmd_diff(int argc, char *argv[])
char *header = NULL;
if (asprintf(&header, "diffstat %s %s",
- labels[0], labels[1]) == -1)
+ labels[0], labels[1]) == -1) {
+ error = got_error_from_errno("asprintf");
goto done;
+ }
error = print_diffstat(&dsa, &diffstat_paths, header);
free(header);
--
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
got: cleanup failure leaks and set error on asprintf() failure