From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gotwebd: openat(2) and less params for scraping repo info
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Tue, 22 Nov 2022 19:23:17 +0100

On Tue, Nov 22, 2022 at 12:07:49PM +0100, Omar Polo wrote:
> -	if (asprintf(&d_file, "%s/description", dir) == -1)
> -		return got_error_from_errno("asprintf");
> -
> -	f = fopen(d_file, "r");
> -	if (f == NULL) {
> +	fd = openat(dir, "description", O_RDONLY);
> +	if (fd == -1) {
>  		if (errno != ENOENT && errno != EACCES)
> -			error = got_error_from_errno2("fopen", d_file);
> +			error = got_error_from_errno2("openat", "description");

The error message previously contained the whole path.
It might be more useful to see an absolute path in error logs.
If you want to retain this you could pass both the path to the directory
and the open fd, and then use got_error_from_errno_fmt() here.

Same applies to gotweb_get_clone_url().

>  		goto done;
>  	}
>  
> -	if (fseek(f, 0, SEEK_END) == -1) {
> -		error = got_ferror(f, GOT_ERR_IO);
> -		goto done;
> -	}
> -	len = ftell(f);
> +	len = lseek(fd, 0, SEEK_END);
>  	if (len == -1) {
> -		error = got_ferror(f, GOT_ERR_IO);
> +		error = got_error_from_errno("lseek");
>  		goto done;

This could instead use fstat() and read sb.st_size. That is what
we usually do and avoids having to seek back to the beginning below.

Same applies to gotweb_get_clone_url().

>  	}
>  
> -	if (len == 0) {
> -		*description = strdup("");
> -		if (*description == NULL)
> -			error = got_error_from_errno("strdup");
> +	if (lseek(fd, 0, SEEK_SET) == -1) {
> +		error = got_error_from_errno("fseek");
>  		goto done;
>  	}
>  
> -	if (fseek(f, 0, SEEK_SET) == -1) {
> -		error = got_ferror(f, GOT_ERR_IO);
> -		goto done;
> -	}
> +	if (len > SIZE_MAX - 1)
> +		len = SIZE_MAX - 1;
> +