"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
gotwebd: drop needless qs NULL check
To:
gameoftrees@openbsd.org
Date:
Wed, 21 Sep 2022 20:00:07 +0200

Download raw body.

Thread
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, "</div>\n"	/* #site_path */
	 	    "</div>\n"			/* #site_link */
	 	    "<div id='content'>\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, "</div>\n"	/* #site_path */