On Fri, May 31, 2024 at 08:14:58PM +1000, Mark Jamsek wrote:
> Kyle's right in that we leak this lines array because it gets allocated
> in diffreg.c:got_diffreg_output() but stsp's right in that the problem
> is elsewhere with some incorrect NULL pointer checks.
> 
> The below diff plugs the leak for me; can you please confirm it does for
> you, too, Kyle?

It certainly helps here. ok stsp@, thank you!

> diff /home/mark/src/got
> commit - c6458e88f5a9085ec9206a60b93a713138b9b2fa
> path + /home/mark/src/got
> blob - 245df76cba6ccd1d6c155ecbb3632f386db7f3e1
> file + lib/diff.c
> --- lib/diff.c
> +++ lib/diff.c
> @@ -146,12 +146,15 @@ diff_blobs(struct got_diff_line **lines, size_t *nline
>  	off_t outoff = 0;
>  	int n;
>  
> -	if (lines && *lines && *nlines > 0)
> -		outoff = (*lines)[*nlines - 1].offset;
> -	else if (lines) {
> -		err = add_line_metadata(lines, nlines, 0, GOT_DIFF_LINE_NONE);
> -		if (err)
> -			goto done;
> +	if (lines && *lines) {
> +		if (*nlines > 0)
> +			outoff = (*lines)[*nlines - 1].offset;
> +		else {
> +			err = add_line_metadata(lines, nlines,
> +			    0, GOT_DIFF_LINE_NONE);
> +			if (err != NULL)
> +				goto done;
> +		}
>  	}
>  
>  	if (resultp)
> @@ -218,7 +221,7 @@ diff_blobs(struct got_diff_line **lines, size_t *nline
>  		if (n < 0)
>  			goto done;
>  		outoff += n;
> -		if (lines) {
> +		if (lines && *lines) {
>  			err = add_line_metadata(lines, nlines, outoff,
>  			    GOT_DIFF_LINE_BLOB_MIN);
>  			if (err)
> @@ -230,7 +233,7 @@ diff_blobs(struct got_diff_line **lines, size_t *nline
>  		if (n < 0)
>  			goto done;
>  		outoff += n;
> -		if (lines) {
> +		if (lines && *lines) {
>  			err = add_line_metadata(lines, nlines, outoff,
>  			    GOT_DIFF_LINE_BLOB_PLUS);
>  			if (err)
> blob - 5f1c310ce05497519a9838e46b29d28af595d0b4
> file + lib/diffreg.c
> --- lib/diffreg.c
> +++ lib/diffreg.c
> @@ -272,13 +272,13 @@ got_diffreg_output(struct got_diff_line **lines, size_
>  	switch (output_format) {
>  	case GOT_DIFF_OUTPUT_UNIDIFF:
>  		rc = diff_output_unidiff(
> -		    lines ? &output_info : NULL, outfile, &info,
> +		    lines && *lines ? &output_info : NULL, outfile, &info,
>  		    diff_result->result, context_lines);
>  		if (rc != DIFF_RC_OK)
>  			return got_error_set_errno(rc, "diff_output_unidiff");
>  		break;
>  	case GOT_DIFF_OUTPUT_PLAIN:
> -		rc = diff_output_plain(lines ? &output_info : NULL,
> +		rc = diff_output_plain(lines && *lines ? &output_info : NULL,
>  		    outfile, &info, diff_result->result, 1);
>  		if (rc != DIFF_RC_OK)
>  			return got_error_set_errno(rc, "diff_output_edscript");
> 
> 
> -- 
> Mark Jamsek <https://bsdbox.org>
> GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68