From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gotwebd: error path leak & semplification for render_index
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Sat, 5 Nov 2022 14:30:50 +0100

On Sat, Nov 05, 2022 at 12:42:27PM +0100, Omar Polo wrote:
> I'm still trying to migrate gotwebd to that templating system I posted
> some time ago and so I'm re-reading all the code that produces the
> html...
> 
> There's a memory leak in the error path if asprintf fails, and while
> here I'd also like to simplify the check and remove the `goto render'.

Looks good to me, just one hint below:

> -		if (lstat(repo_dir->path, &st) == 0 &&
> -		    S_ISDIR(st.st_mode) &&
> -		    !got_path_dir_is_empty(repo_dir->path))
> -			goto render;
> -		else {
> +		if (lstat(repo_dir->path, &st) == -1 ||
> +		    !S_ISDIR(st.st_mode) ||
> +		    got_path_dir_is_empty(repo_dir->path)) {

It might be a good idea to set err = got_error_from_errno() if lstat() fails
for some reason other than ENOENT, and do the ISDIR + empty checks separately.

>  			gotweb_free_repo_dir(repo_dir);
>  			repo_dir = NULL;
>  			continue;
>  		}
> -render:
> +
>  		d_disp++;
>  		t->prev_disp++;
>  
> 
>