On Mon, May 27, 2024 at 11:58:02PM -0500, Kyle Ackerman wrote:
> Hello all,
> 
> There is a memory leak within `got diff` that leaks line metadata.
> 
> ******** Start dump got *******
> M=8 I=1 F=1 U=1 J=2 R=0 X=0 C=0x449af07a cache=0 G=4096
> Leak report:
>                  f     sum      #    avg
>      0xd551a917735     512      1    512 addr2line -e /home/kyle/bin/got 0x4e735
>      0xd57f17f70b0      64      1     64 addr2line -e /usr/lib/libc.so.100.1 0xa40b0
>      0xd57f182c4d1     112      7     16 addr2line -e /usr/lib/libc.so.100.1 0xd94d1
>      0xd57f18158d2    1024      1   1024 addr2line -e /usr/lib/libc.so.100.1 0xc28d2
>      0xd57f17aec53   69632      1  69632 addr2line -e /usr/lib/libc.so.100.1 0x5bc53
> 
> ******** End dump got *******

Could you please show what these addr2line commands are printing?

> Here is the diff to patch the memory leak
> 
> diff /home/kyle/src/got
> commit - c89c70b628c1825024e333214392011409d71184
> path + /home/kyle/src/got
> blob - 245df76cba6ccd1d6c155ecbb3632f386db7f3e1
> file + lib/diff.c
> --- lib/diff.c
> +++ lib/diff.c
> @@ -1282,6 +1282,8 @@ diff_objects_as_trees(struct got_diff_line **lines, si
>  	if (want_linemeta) {
>  		*lines = arg.lines; /* was likely re-allocated */
>  		*nlines = arg.nlines;
> +	} else {
> +		free(arg.lines);
>  	}
>  done:
>  	if (tree1)

If this fixes the above leak report then I wonder if the real problem isn't
elsewhere. As far as I understand, arg.lines is set to NULL when want_linemeta
is false. And if arg.lines is NULL, the expectation is that no line offset
information will be produced by the diff driver and callbacks, and args.lines
will remain as NULL and won't need to be freed.

Is there code somewhere which reallocs args.lines even though it is NULL?
This would be where a fix is needed, I believe. I cannot spot such code
in lib/diff.c. Can you perhaps pin-point something?