From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: gotwebd: drop index_page_str and page_str
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Fri, 16 Dec 2022 15:11:26 -0700

On Fri, Dec 16, 2022 at 09:24:17PM +0100, Omar Polo wrote:
> I noticed the warnings from the linker about the usage of sprintf and
> so I looked into it.
> 
> We're using sprintf to update index_page_str and page_str if they were
> negative.  While as far as I can tell this specific usage was safe
> (we're guaranteed that the allocated string is at least one byte + NUL
> terminator long) I'd like to drop it.
> 
> There are multiple ways to fix it but I went with the most fun one:
> killing the fields as they're never read from; this saves an
> allocation too.
> 
> ok?

ok

> 
> 
> diff /home/op/w/got
> commit - 2b94f9b9fd020b04ccadec8136e0e8fe06a3c8fe
> path + /home/op/w/got
> blob - 457891c2c6c6e2e695f33d292ea9e53fefdefb7c
> file + gotwebd/gotweb.c
> --- gotwebd/gotweb.c
> +++ gotwebd/gotweb.c
> @@ -355,7 +355,6 @@ gotweb_init_querystring(struct querystring **qs)
>  	(*qs)->file = NULL;
>  	(*qs)->folder = NULL;
>  	(*qs)->index_page = 0;
> -	(*qs)->index_page_str = NULL;
>  	(*qs)->path = NULL;
>  
>  	return error;
> @@ -522,12 +521,6 @@ qa_found:
>  		case INDEX_PAGE:
>  			if (strlen(value) == 0)
>  				break;
> -			(*qs)->index_page_str = strdup(value);
> -			if ((*qs)->index_page_str == NULL) {
> -				error = got_error_from_errno2("%s: strdup",
> -				    __func__);
> -				goto done;
> -			}
>  			(*qs)->index_page = strtonum(value, INT64_MIN,
>  			    INT64_MAX, &errstr);
>  			if (errstr) {
> @@ -535,10 +528,8 @@ qa_found:
>  				    __func__, errstr);
>  				goto done;
>  			}
> -			if ((*qs)->index_page < 0) {
> +			if ((*qs)->index_page < 0)
>  				(*qs)->index_page = 0;
> -				sprintf((*qs)->index_page_str, "%d", 0);
> -			}
>  			break;
>  		case PATH:
>  			(*qs)->path = strdup(value);
> @@ -551,12 +542,6 @@ qa_found:
>  		case PAGE:
>  			if (strlen(value) == 0)
>  				break;
> -			(*qs)->page_str = strdup(value);
> -			if ((*qs)->page_str == NULL) {
> -				error = got_error_from_errno2("%s: strdup",
> -				    __func__);
> -				goto done;
> -			}
>  			(*qs)->page = strtonum(value, INT64_MIN,
>  			    INT64_MAX, &errstr);
>  			if (errstr) {
> @@ -564,10 +549,8 @@ qa_found:
>  				    __func__, errstr);
>  				goto done;
>  			}
> -			if ((*qs)->page < 0) {
> +			if ((*qs)->page < 0)
>  				(*qs)->page = 0;
> -				sprintf((*qs)->page_str, "%d", 0);
> -			}
>  			break;
>  		default:
>  			break;
> @@ -614,9 +597,7 @@ gotweb_free_querystring(struct querystring *qs)
>  		free(qs->file);
>  		free(qs->folder);
>  		free(qs->headref);
> -		free(qs->index_page_str);
>  		free(qs->path);
> -		free(qs->page_str);
>  	}
>  	free(qs);
>  }
> blob - 64fce7c6d8ee3f57305034f0c0434cff991a82bd
> file + gotwebd/gotwebd.h
> --- gotwebd/gotwebd.h
> +++ gotwebd/gotwebd.h
> @@ -373,10 +373,8 @@ struct querystring {
>  	char		*folder;
>  	char		*headref;
>  	int		 index_page;
> -	char		*index_page_str;
>  	char		*path;
>  	int		 page;
> -	char		*page_str;
>  };
>  
>  struct querystring_keys {
> 

-- 

Tracey Emery