[2/2] log: UNLEAK original pending objects

Commit Message

Andrzej Hunt Sept. 18, 2021, 1:49 p.m. UTC

From: Andrzej Hunt <[e-mail]>

cmd_show() uses objects to point to rev.pending's objects. Later, it might
detach rev.pending's objects: when handling an OBJ_COMMIT it will reset
rev.pending (without freeing its objects). Detaching (as opposed to freeing)
is necessary because cmd_show() continues iterating over the original objects
array.

We choose to UNLEAK because there's no real advantage to cleaning up
properly (cmd_show() exits immediately after looping over these
objects). A number of alternatives exist, but are all significantly more
complex for no gain:

Alternative 1:
  Convert objects into an object_array, and memcpy rev.pending into it
  (followed by detaching rev.pending immediately - making objects the
  owner of what used to be rev.pending). Then we could safely
  objects_array_clear() at the end of cmd_show(). And we can rely on
  a preexisting UNLEAK(rev) to avoid having to clean up rev.pending.
  This is a more complex and riskier approach vs a simple UNLEAK,
  and doesn't add any user-visible value.

Alternative 2:
  A variation on alternative 1. We make objects own the object_array as
  before. Once we're done, we free the new rev.pending array (which
  might be empty), and we memcpy objects back into rev.pending, relying
  on the existin UNLEAK(rev) to avoid having to free rev.pending.

ASAN output from t0000:

Direct leak of 41 byte(s) in 1 object(s) allocated from:
    #0 0x487504 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:437:3
    #1 0x9e4ef8 in xstrdup wrapper.c:29:14
    #2 0x86395d in add_object_array_with_path object.c:366:17
    #3 0x9264fc in add_pending_object_with_path revision.c:330:2
    #4 0x91c4e0 in handle_revision_arg_1 revision.c:2086:2
    #5 0x91bfcd in handle_revision_arg revision.c:2093:12
    #6 0x91ff5a in setup_revisions revision.c:2780:7
    #7 0x5a7678 in cmd_log_init_finish builtin/log.c:206:9
    #8 0x5a4f18 in cmd_log_init builtin/log.c:278:2
    #9 0x5a55d1 in cmd_show builtin/log.c:646:2
    #10 0x4cff30 in run_builtin git.c:461:11
    #11 0x4cdb00 in handle_builtin git.c:713:3
    #12 0x4cf527 in run_argv git.c:780:4
    #13 0x4cd426 in cmd_main git.c:911:19
    #14 0x6b2eb5 in main common-main.c:52:11
    #15 0x7f74fc9bd349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: 41 byte(s) leaked in 1 allocation(s).

Signed-off-by: Andrzej Hunt <[e-mail]>
---
 builtin/log.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Patch

diff --git a/builtin/log.c b/builtin/log.c
index 6faaddf17a6..769ee6a9258 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -702,7 +702,8 @@  int cmd_show(int argc, const char **argv, const char *prefix)
 			ret = error(_("unknown type: %d"), o->type);
 		}
 	}
-	free(objects);
+	UNLEAK(objects);
+
 	return ret;
 }