From: Stefan Sperling Subject: Re: got: parse.y: Remove superfluous strdup(3) calls To: Tracey Emery Cc: Martin Vahlensieck , gameoftrees@openbsd.org Date: Tue, 31 Aug 2021 09:29:05 +0200 On Mon, Aug 30, 2021 at 03:32:39PM -0600, Tracey Emery wrote: > 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? I'm in favour of this change. Fewer lines of code getting the same job done. What's not to like? > > 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 > >