On Thu, Oct 03, 2024 at 11:58:42AM +0000, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <stolee@xxxxxxxxx> > > 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. Thanks a bunch for fixing this! The change looks good to me. Patrick
