From: Stefan Sperling <stsp@stsp.name>
Subject: Re: avoid allocating too much errors in cmd_info
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Fri, 5 Aug 2022 12:22:54 +0200

On Fri, Aug 05, 2022 at 12:07:59PM +0200, Omar Polo wrote:
> functions like got_error_path use `get_custom_err' to get a statically
> allocated struct got_custom_error that's on static array of errors (used
> as a ring.)  it's nice and simple, but the drawbacks is that custom
> errors shouldn't be used "for too long" because they can be overwritten.
> 
> One example of such is `got info':
> 
> 	% got info $(jot 30)
> 	work tree: /home/op/w/got
> 	work tree base commit: 6adac3028e24cdb048ee573633ce056b74a5b360
> 	work tree path prefix: /
> 	work tree branch reference: refs/heads/main
> 	work tree UUID: e873031f-1f7e-42b7-96b4-04afb872b83a
> 	repository: /home/op/git/got.git
> 	got: 24: bad path
> 
> where i would have expected "got: 1: bad path".  it's a nitpick, but
> it's also easy to fix.  ok?
> 
> diff /home/op/w/got
> commit - 575dd20f850659c3caa159936f3fa7e88867b8a7
> path + /home/op/w/got
> blob - 77ab232b3e3dd99d8500f2e4792976396ecafda9
> file + got/got.c
> --- got/got.c
> +++ got/got.c
> @@ -13095,8 +13095,7 @@ cmd_info(int argc, char *argv[])
>  			 * Assume this path will fail. This will be corrected
>  			 * in print_path_info() in case the path does suceeed.
>  			 */
> -			pe->data = (void *)got_error_path(pe->path,
> -			    GOT_ERR_BAD_PATH);
> +			pe->data = (void *)got_error(GOT_ERR_BAD_PATH);
>  		}
>  		error = got_worktree_path_info(worktree, &paths,
>  		    print_path_info, &paths, check_cancelled, NULL);
> @@ -13104,7 +13103,8 @@ cmd_info(int argc, char *argv[])
>  			goto done;
>  		TAILQ_FOREACH(pe, &paths, entry) {
>  			if (pe->data != NULL) {
> -				error = pe->data; /* bad path */
> +				error = got_error_path(pe->path,
> +				    GOT_ERR_BAD_PATH);
>  				break;
>  			}
>  		}

It's a bit strange that pe->data is now effectively just used as a flag.
Do we have a way of treating pe->data as an error pointer, and then
append path information? Maybe something like this would work?

	if (pe->data != NULL) {
		int code = ((const struct got_error *)pe->data)->code;
		error = got_error_fmt(code, "%s", pe->path)
		break;
	}