Skip to content

Commit

Permalink
log: UNLEAK original pending objects
Browse files Browse the repository at this point in the history
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 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
    git#8 0x5a4f18 in cmd_log_init builtin/log.c:278:2
    git#9 0x5a55d1 in cmd_show builtin/log.c:646:2
    git#10 0x4cff30 in run_builtin git.c:461:11
    git#11 0x4cdb00 in handle_builtin git.c:713:3
    git#12 0x4cf527 in run_argv git.c:780:4
    git#13 0x4cd426 in cmd_main git.c:911:19
    git#14 0x6b2eb5 in main common-main.c:52:11
    git#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 <andrzej@ahunt.org>
  • Loading branch information
ahunt committed Sep 18, 2021
1 parent e3a4f5b commit da21755
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion builtin/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit da21755

Please sign in to comment.