From: Omar Polo Subject: gotwebd: drop needless qs NULL check To: gameoftrees@openbsd.org Date: Wed, 21 Sep 2022 20:00:07 +0200 spotted while doing something else. If qs is NULL we would have already crashed and a couple of lines before we do dereference it. Every time we hit gotweb_render_header we have already gone thru a (successfull) gotweb_init_querystring, otherwise we would have `goto err' in gotweb_process_request. got diff -w with extra context for ease of review: --- gotwebd/gotweb.c +++ gotwebd/gotweb.c @@ -740,11 +740,10 @@ gotweb_render_header(struct request *c) c->script_name, srv->logo, qs->index_page, srv->site_link); if (r == -1) goto done; - if (qs != NULL) { if (qs->path != NULL) { char *epath; if (fcgi_printf(c, " / ") == -1) goto done; @@ -793,11 +792,10 @@ gotweb_render_header(struct request *c) } if (fcgi_printf(c, " / %s", action) == -1) goto done; } - } fcgi_printf(c, "\n" /* #site_path */ "\n" /* #site_link */ "
\n"); and the real diff for got patch :) (admittedly only build-tested.) diff /home/op/w/gwd-template commit - 611e5fc2074d428e17f920dc595496af4dd0dc77 path + /home/op/w/gwd-template blob - aafa0224a0f81173ac396f4607674dc80127339a file + gotwebd/gotweb.c --- gotwebd/gotweb.c +++ gotwebd/gotweb.c @@ -742,59 +742,57 @@ gotweb_render_header(struct request *c) if (r == -1) goto done; - if (qs != NULL) { - if (qs->path != NULL) { - char *epath; + if (qs->path != NULL) { + char *epath; - if (fcgi_printf(c, " / ") == -1) - goto done; + if (fcgi_printf(c, " / ") == -1) + goto done; - err = gotweb_escape_html(&epath, qs->path); - if (err) - return err; - r = gotweb_link(c, &(struct gotweb_url){ - .action = SUMMARY, - .index_page = -1, - .page = -1, - .path = qs->path, - }, "%s", epath); - free(epath); - if (r == -1) - goto done; - } - if (qs->action != INDEX) { - const char *action = ""; + err = gotweb_escape_html(&epath, qs->path); + if (err) + return err; + r = gotweb_link(c, &(struct gotweb_url){ + .action = SUMMARY, + .index_page = -1, + .page = -1, + .path = qs->path, + }, "%s", epath); + free(epath); + if (r == -1) + goto done; + } + if (qs->action != INDEX) { + const char *action = ""; - switch (qs->action) { - case BLAME: - action = "blame"; - break; - case BRIEFS: - action = "briefs"; - break; - case COMMITS: - action = "commits"; - break; - case DIFF: - action = "diff"; - break; - case SUMMARY: - action = "summary"; - break; - case TAG: - action = "tag"; - break; - case TAGS: - action = "tags"; - break; - case TREE: - action = "tree"; - break; - } - - if (fcgi_printf(c, " / %s", action) == -1) - goto done; + switch (qs->action) { + case BLAME: + action = "blame"; + break; + case BRIEFS: + action = "briefs"; + break; + case COMMITS: + action = "commits"; + break; + case DIFF: + action = "diff"; + break; + case SUMMARY: + action = "summary"; + break; + case TAG: + action = "tag"; + break; + case TAGS: + action = "tags"; + break; + case TREE: + action = "tree"; + break; } + + if (fcgi_printf(c, " / %s", action) == -1) + goto done; } fcgi_printf(c, "
\n" /* #site_path */