]> git.ipfire.org Git - thirdparty/git.git/commit
line-log: protect inner strbuf from free
authorDerrick Stolee <stolee@gmail.com>
Thu, 3 Oct 2024 11:58:42 +0000 (11:58 +0000)
committerJunio C Hamano <gitster@pobox.com>
Thu, 3 Oct 2024 16:07:16 +0000 (09:07 -0700)
commitfc5589d6c1200f87689ff95067f18caa2a826382
treea56a39b09739852a36ed31e105795a57ed32d7ee
parent4f71522dfb7fc53eff569023303980c66114b1bc
line-log: protect inner strbuf from free

The output_prefix() method in line-log.c may call a function pointer via
the diff_options struct. This function pointer returns a strbuf struct
and then its buffer is passed back. However, that implies that the
consumer is responsible to free the string. This is especially true
because the default behavior is to duplicate the empty string.

The existing functions used in the output_prefix pointer include:

 1. idiff_prefix_cb() in diff-lib.c. This returns the data pointer, so
    the value exists across multiple calls.

 2. diff_output_prefix_callback() in graph.c. This uses a static strbuf
    struct, so it reuses buffers across calls. These should not be
    freed.

 3. output_prefix_cb() in range-diff.c. This is similar to the
    diff-lib.c case.

In each case, we should not be freeing this buffer. We can convert the
output_prefix() function to return a const char pointer and stop freeing
the result.

This choice is essentially the opposite of what was done in 394affd46d
(line-log: always allocate the output prefix, 2024-06-07).

This was discovered via 'valgrind' while investigating a public report
of a bug in 'git log --graph -L' [1].

[1] https://github.com/git-for-windows/git/issues/5185

This issue would have been caught by the new test, when Git is compiled
with ASan to catch these double frees.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
line-log.c
t/t4211-line-log.sh