From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: got: parse.y: Remove superfluous strdup(3) calls
To: Martin Vahlensieck <openbsd@academicsolutions.ch>
Cc: gameoftrees@openbsd.org
Date: Mon, 30 Aug 2021 15:32:39 -0600

On Mon, Aug 30, 2021 at 10:00:41PM +0200, Martin Vahlensieck wrote:
> Hi
> 
> In the grammar (for gotweb and got-read-gotconfig) $2 points to a
> freshly allocated string.  This string is then duplicated and the
> original ($2) is freed.  Simplify this by assigning $2 directly.  Also
> something similar happens in yyerror, so this is changed as well.
> 
> Or am I overlooking something?
> 
> Best,
> 
> Martin
> 

I don't think the diff is wrong. I've just often waffled over the two
implementations. I'm fine with this change. Anyone else care to comment?

> diff 1dd93b2a37823ef322c22aa1a8ff94c53fb25186 /home/puffy/test_conf
> blob - 2d9864e81b2f1c9a6adb6601a6a49a023d906ba6
> file + gotweb/parse.y
> --- gotweb/parse.y
> +++ gotweb/parse.y
> @@ -118,71 +118,29 @@ boolean		: STRING {
>  		}
>  		;
>  main		: GOT_REPOS_PATH STRING {
> -			gw_conf->got_repos_path = strdup($2);
> -			if (gw_conf->got_repos_path == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +			gw_conf->got_repos_path = $2;
>  		}
>  		| GOT_WWW_PATH STRING {
> -			gw_conf->got_www_path = strdup($2);
> -			if (gw_conf->got_www_path == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +			gw_conf->got_www_path = $2;
>  		}
>  		| GOT_MAX_REPOS NUMBER {
>  			if ($2 > 0)
>  				gw_conf->got_max_repos = $2;
>  		}
>  		| GOT_SITE_NAME STRING {
> -			gw_conf->got_site_name = strdup($2);
> -			if (gw_conf->got_site_name == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +			gw_conf->got_site_name = $2;
>  		}
>  		| GOT_SITE_OWNER STRING {
> -			gw_conf->got_site_owner = strdup($2);
> -			if (gw_conf->got_site_owner == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +			gw_conf->got_site_owner = $2;
>  		}
>  		| GOT_SITE_LINK STRING {
> -			gw_conf->got_site_link = strdup($2);
> -			if (gw_conf->got_site_link == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +			gw_conf->got_site_link = $2;
>  		}
>  		| GOT_LOGO STRING {
> -			gw_conf->got_logo = strdup($2);
> -			if (gw_conf->got_logo== NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +			gw_conf->got_logo = $2;
>  		}
>  		| GOT_LOGO_URL STRING {
> -			gw_conf->got_logo_url = strdup($2);
> -			if (gw_conf->got_logo_url== NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +			gw_conf->got_logo_url = $2;
>  		}
>  		| GOT_SHOW_SITE_OWNER boolean {
>  			gw_conf->got_show_site_owner = $2;
> @@ -232,9 +190,8 @@ yyerror(const char *fmt, ...)
>  		gerror = got_error_from_errno("asprintf");
>  		return(0);
>  	}
> -	gerror = got_error_msg(GOT_ERR_PARSE_CONFIG, strdup(err));
> +	gerror = got_error_msg(GOT_ERR_PARSE_CONFIG, err);
>  	free(msg);
> -	free(err);
>  	return(0);
>  }
>  
> blob - f1fe63531f72a34ed515e9fb73cfe9c8c377d1b9
> file + libexec/got-read-gotconfig/parse.y
> --- libexec/got-read-gotconfig/parse.y
> +++ libexec/got-read-gotconfig/parse.y
> @@ -192,31 +192,13 @@ remoteopts2	: remoteopts2 remoteopts1 nl
>  	   	| remoteopts1 optnl
>  		;
>  remoteopts1	: REPOSITORY STRING {
> -	   		remote->repository = strdup($2);
> -			if (remote->repository == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +	   		remote->repository = $2;
>  	   	}
>  	   	| SERVER STRING {
> -	   		remote->server = strdup($2);
> -			if (remote->server == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +	   		remote->server = $2;
>  		}
>  		| PROTOCOL STRING {
> -	   		remote->protocol = strdup($2);
> -			if (remote->protocol == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +	   		remote->protocol = $2;
>  		}
>  		| MIRROR_REFERENCES boolean {
>  			remote->mirror_references = $2;
> @@ -267,31 +249,13 @@ fetchopts2	: fetchopts2 fetchopts1 nl
>  	   	| fetchopts1 optnl
>  		;
>  fetchopts1	: REPOSITORY STRING {
> -	   		remote->fetch_config->repository = strdup($2);
> -			if (remote->fetch_config->repository == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +	   		remote->fetch_config->repository = $2;
>  	   	}
>  	   	| SERVER STRING {
> -	   		remote->fetch_config->server = strdup($2);
> -			if (remote->fetch_config->server == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +	   		remote->fetch_config->server = $2;
>  		}
>  		| PROTOCOL STRING {
> -	   		remote->fetch_config->protocol = strdup($2);
> -			if (remote->fetch_config->protocol == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +	   		remote->fetch_config->protocol = $2;
>  		}
>  		| PORT portplain {
>  			remote->fetch_config->port = $2;
> @@ -307,31 +271,13 @@ sendopts2	: sendopts2 sendopts1 nl
>  	   	| sendopts1 optnl
>  		;
>  sendopts1	: REPOSITORY STRING {
> -	   		remote->send_config->repository = strdup($2);
> -			if (remote->send_config->repository == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +	   		remote->send_config->repository = $2;
>  	   	}
>  	   	| SERVER STRING {
> -	   		remote->send_config->server = strdup($2);
> -			if (remote->send_config->server == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +	   		remote->send_config->server = $2;
>  		}
>  		| PROTOCOL STRING {
> -	   		remote->send_config->protocol = strdup($2);
> -			if (remote->send_config->protocol == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +	   		remote->send_config->protocol = $2;
>  		}
>  		| PORT portplain {
>  			remote->send_config->port = $2;
> @@ -349,26 +295,14 @@ remote		: REMOTE STRING {
>  				yyerror("%s", error->msg);
>  				YYERROR;
>  			}
> -			remote->name = strdup($2);
> -			if (remote->name == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +			remote->name = $2;
>  		} '{' optnl remoteopts2 '}' {
>  			TAILQ_INSERT_TAIL(&gotconfig.remotes, remote, entry);
>  			gotconfig.nremotes++;
>  		}
>  		;
>  author		: AUTHOR STRING {
> -	   		gotconfig.author = strdup($2);
> -			if (gotconfig.author == NULL) {
> -				free($2);
> -				yyerror("strdup");
> -				YYERROR;
> -			}
> -			free($2);
> +	   		gotconfig.author = $2;
>  		}
>  		;
>  optnl		: '\n' optnl
> @@ -404,9 +338,8 @@ yyerror(const char *fmt, ...)
>  		gerror = got_error_from_errno("asprintf");
>  		return(0);
>  	}
> -	gerror = got_error_msg(GOT_ERR_PARSE_CONFIG, strdup(err));
> +	gerror = got_error_msg(GOT_ERR_PARSE_CONFIG, err);
>  	free(msg);
> -	free(err);
>  	return(0);
>  }
>  int

-- 

Tracey Emery