From: Stefan Sperling <stsp@stsp.name>
Subject: Re: in diff error message, say what was being diffed
To: James Cook <falsifian@falsifian.org>
Cc: gameoftrees@openbsd.org
Date: Sat, 16 Sep 2023 20:16:34 +0200

On Sat, Sep 16, 2023 at 04:22:36PM +0000, James Cook wrote:
> Recently I ran "got ci" to add a large number (>100 000) files and got the
> error:
> 	got: diff_atomize_file: Cannot allocate memory
> 
> It turns out one of the files was really big. This diff changes the error to
> say:
> 	got: /dev/null vs /path/to/the/really/big/file: diff_atomize_file: Cannot allocate memory
> 
> Maybe it's to verbose and/or unneccesary. Even with the original error it
> was not hard to guess that "got ci -n" would fix the problem. However, this
> error would have saved me a bit of time because I ended up trying to
> understand why adding a large number of files
> would trigger an allocation error (memory leak?), and only deep into
> debugging did I realize it was one big file causing the problem.
> 
> -- 
> James
> 
> 
> diff /home/falsifian/co/got
> commit - 19a5edf3da4acec32a964ac91c065e121b0a0ec2
> path + /home/falsifian/co/got
> blob - f448440ff5880b5d6ec7ec9b7d45ef85b47ee2e6
> file + lib/diff.c
> --- lib/diff.c
> +++ lib/diff.c
> @@ -378,8 +378,15 @@ diff_blob_file(struct got_diffreg_result **resultp,
>  	err = got_diffreg(&result, f1, f2, diff_algo, ignore_whitespace,
>  	    force_text_diff);
> -	if (err)
> +	if (err) {
> +		char msg[GOT_ERR_MAX_MSG_SIZE];
> +		if (snprintf(msg, sizeof(msg), "%s vs %s: %s",
> +		             label1 ? label1 : idstr1,
> +		             f2_exists ? label2 : "/dev/null", err->msg) >= 0) {
> +			err = got_error_msg(err->code, msg);
> +		}
>  		goto done;
> +	}

Could you use got_error_fmt() instead of snprintf + got_error_msg?

I am happy with this change. As a follow-up change, should we try to detect
this ENOMEM condition in got commit and proceed without showing a diff?

>  	if (outfile) {
>  		err = got_diffreg_output(NULL, NULL, result,
> 
>