"GOT", but the "O" is a cute, smiling sun Index | Thread

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: gotwebd address list head allocation
To:
"Todd C. Miller" <millert@openbsd.org>
Cc:
gameoftrees@openbsd.org
Date:
Tue, 16 Aug 2022 16:50:04 +0200

Download raw body.

On Tue, Aug 16, 2022 at 08:15:07AM -0600, Todd C. Miller wrote:
> On Tue, 16 Aug 2022 14:15:54 +0200, Stefan Sperling wrote:
> 
> > gotwebd allocates address list heads separately, which is unnecessary.
> > We only use one such head per struct server / struct server_conf.
> > we can simply inline storage for address list heads in the structs
> > which contain them.
> 
> That makes a lot of sense.  OK millert@
> 
>  - todd
> 
> 

Thanks, committed.

And this patch performs the same mechanical change for the server and
socket list heads in struct gotwebd. ok?

diff e087e1f6e7b926dcff23d90de416399c114d582e ccda34b216de173ac2bca2c2f9b3aa94aabdbddf
commit - e087e1f6e7b926dcff23d90de416399c114d582e
commit + ccda34b216de173ac2bca2c2f9b3aa94aabdbddf
blob - b53d5eff71873cc7e138a6b88fc6f313800c0701
blob + 2ff6fae01c54ac33ce96e4a1a48f018b553cdc75
--- gotwebd/config.c
+++ gotwebd/config.c
@@ -58,14 +58,8 @@ config_init(struct gotwebd *env)
 	what = ps->ps_what[privsep_process];
 	if (what & CONFIG_SOCKS) {
 		env->server_cnt = 0;
-		env->servers = calloc(1, sizeof(*env->servers));
-		if (env->servers == NULL)
-			fatalx("%s: calloc", __func__);
-		env->sockets = calloc(1, sizeof(*env->sockets));
-		if (env->sockets == NULL)
-			fatalx("%s: calloc", __func__);
-		TAILQ_INIT(env->servers);
-		TAILQ_INIT(env->sockets);
+		TAILQ_INIT(&env->servers);
+		TAILQ_INIT(&env->sockets);
 	}
 	 return 0;
 }
@@ -116,7 +110,7 @@ config_getserver(struct gotwebd *env, struct imsg *ims
 	    srv->name, srv->fcgi_socket ? "yes" : "no", srv->unix_socket ?
 	    "yes" : "no");
 
-	TAILQ_INSERT_TAIL(env->servers, srv, entry);
+	TAILQ_INSERT_TAIL(&env->servers, srv, entry);
 
 	 return 0;
 }
@@ -209,7 +203,7 @@ config_getsock(struct gotwebd *env, struct imsg *imsg)
 	memcpy(&sock->conf, &sock_conf, sizeof(sock->conf));
 	sock->fd = imsg->fd;
 
-	TAILQ_INSERT_TAIL(env->sockets, sock, entry);
+	TAILQ_INSERT_TAIL(&env->sockets, sock, entry);
 
 	for (i = 0; i < PRIV_FDS__MAX; i++)
 		sock->priv_fd[i] = -1;
@@ -299,7 +293,7 @@ config_getfd(struct gotwebd *env, struct imsg *imsg)
 	IMSG_SIZE_CHECK(imsg, &sock_id);
 	memcpy(&sock_id, p, sizeof(sock_id));
 
-	TAILQ_FOREACH(sock, env->sockets, entry) {
+	TAILQ_FOREACH(sock, &env->sockets, entry) {
 		const int nfds = (GOTWEB_PACK_NUM_TEMPFILES + PRIV_FDS__MAX);
 		for (i = 0; i < nfds; i++) {
 			if (i < PRIV_FDS__MAX && sock->priv_fd[i] == -1) {
blob - 7d71f84f29cba62f56825341dc5b7e470b9a6684
blob + 9a24cf057d5612912ba4c4370d1cd749d8cdddcd
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -308,24 +308,24 @@ gotweb_get_server(uint8_t *server_name, uint8_t *docum
 
 	/* check against document_root first */
 	if (strlen(server_name) > 0)
-		TAILQ_FOREACH(srv, gotwebd_env->servers, entry)
+		TAILQ_FOREACH(srv, &gotwebd_env->servers, entry)
 			if (strcmp(srv->name, server_name) == 0)
 				goto done;
 
 	/* check against document_root second */
 	if (strlen(document_root) > 0)
-		TAILQ_FOREACH(srv, gotwebd_env->servers, entry)
+		TAILQ_FOREACH(srv, &gotwebd_env->servers, entry)
 			if (strcmp(srv->name, document_root) == 0)
 				goto done;
 
 	/* check against subdomain third */
 	if (strlen(subdomain) > 0)
-		TAILQ_FOREACH(srv, gotwebd_env->servers, entry)
+		TAILQ_FOREACH(srv, &gotwebd_env->servers, entry)
 			if (strcmp(srv->name, subdomain) == 0)
 				goto done;
 
 	/* if those fail, send first server */
-	TAILQ_FOREACH(srv, gotwebd_env->servers, entry)
+	TAILQ_FOREACH(srv, &gotwebd_env->servers, entry)
 		if (srv != NULL)
 			break;
 done:
blob - c6930e3119c2231e948970c004f3f21d33c34c16
blob + 38c11329e49c68c35dbf2decefe120e7c1f83e0d
--- gotwebd/gotwebd.c
+++ gotwebd/gotwebd.c
@@ -292,13 +292,13 @@ gotwebd_configure(struct gotwebd *env)
 	env->gotwebd_reload = env->prefork_gotwebd;
 
 	/* send our gotweb servers */
-	TAILQ_FOREACH(srv, env->servers, entry) {
+	TAILQ_FOREACH(srv, &env->servers, entry) {
 		if (config_setserver(env, srv) == -1)
 			fatalx("%s: send server error", __func__);
 	}
 
 	/* send our sockets */
-	TAILQ_FOREACH(sock, env->sockets, entry) {
+	TAILQ_FOREACH(sock, &env->sockets, entry) {
 		if (config_setsock(env, sock) == -1)
 			fatalx("%s: send socket error", __func__);
 		if (config_setfd(env, sock) == -1)
blob - 2d740d8c68ee194fe07fbfdcd32adc917ba70b44
blob + 0f238f4dcbdecd4cd7a5224cea8dd35c9bd25215
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -328,8 +328,8 @@ struct socket {
 TAILQ_HEAD(socketlist, socket);
 
 struct gotwebd {
-	struct serverlist	*servers;
-	struct socketlist	*sockets;
+	struct serverlist	servers;
+	struct socketlist	sockets;
 
 	struct privsep	*gotwebd_ps;
 	const char	*gotwebd_conffile;
blob - 6a19cc971eac7c808d5e66cf765ae45df9f34677
blob + b1bbd7d1edd47a6c9aec7e31bffa17bcafce1d54
--- gotwebd/parse.y
+++ gotwebd/parse.y
@@ -216,7 +216,7 @@ main		: PREFORK NUMBER {
 server		: SERVER STRING {
 			struct server *srv;
 
-			TAILQ_FOREACH(srv, gotwebd->servers, entry) {
+			TAILQ_FOREACH(srv, &gotwebd->servers, entry) {
 				if (strcmp(srv->name, $2) == 0) {
 					yyerror("server name exists '%s'", $2);
 					free($2);
@@ -239,7 +239,7 @@ server		: SERVER STRING {
 		| SERVER STRING {
 			struct server *srv;
 
-			TAILQ_FOREACH(srv, gotwebd->servers, entry) {
+			TAILQ_FOREACH(srv, &gotwebd->servers, entry) {
 				if (strcmp(srv->name, $2) == 0) {
 					yyerror("server name exists '%s'", $2);
 					free($2);
@@ -397,7 +397,7 @@ socketopts1	: LISTEN ON STRING {
 		| PORT fcgiport {
 			struct server	*srv;
 
-			TAILQ_FOREACH(srv, gotwebd->servers, entry) {
+			TAILQ_FOREACH(srv, &gotwebd->servers, entry) {
 				if (srv->fcgi_socket_port == $2) {
 					yyerror("port already assigned");
 					YYERROR;
@@ -935,7 +935,7 @@ conf_new_server(const char *name)
 	srv->fcgi_socket = gotwebd->fcgi_socket ? gotwebd->fcgi_socket : 0;
 
 	TAILQ_INIT(&srv->al);
-	TAILQ_INSERT_TAIL(gotwebd->servers, srv, entry);
+	TAILQ_INSERT_TAIL(&gotwebd->servers, srv, entry);
 	gotwebd->server_cnt++;
 
 	return srv;
blob - b60e44674a3b5851bb5126437284adc08abea47d
blob + a8601823f7e0c6d8d1c751a2980c6b4976f7003e
--- gotwebd/sockets.c
+++ gotwebd/sockets.c
@@ -123,22 +123,22 @@ sockets_parse_sockets(struct gotwebd *env)
 	struct address *a;
 	int sock_id = 0, ipv4 = 0, ipv6 = 0;
 
-	TAILQ_FOREACH(srv, env->servers, entry) {
+	TAILQ_FOREACH(srv, &env->servers, entry) {
 		if (srv->unix_socket) {
 			sock_id++;
 			new_sock = sockets_conf_new_socket(env, srv,
 			    sock_id, UNIX, 0);
-			TAILQ_INSERT_TAIL(env->sockets, new_sock, entry);
+			TAILQ_INSERT_TAIL(&env->sockets, new_sock, entry);
 		}
 
 		if (srv->fcgi_socket) {
 			sock_id++;
 			new_sock = sockets_conf_new_socket(env, srv,
 			    sock_id, FCGI, 0);
-			TAILQ_INSERT_TAIL(env->sockets, new_sock, entry);
+			TAILQ_INSERT_TAIL(&env->sockets, new_sock, entry);
 
 			/* add ipv6 children */
-			TAILQ_FOREACH(sock, env->sockets, entry) {
+			TAILQ_FOREACH(sock, &env->sockets, entry) {
 				ipv4 = ipv6 = 0;
 
 				TAILQ_FOREACH(a, &sock->conf.al, entry) {
@@ -155,7 +155,7 @@ sockets_parse_sockets(struct gotwebd *env)
 					new_sock = sockets_conf_new_socket(env,
 					    srv, sock_id, FCGI, 1);
 					sockets_dup_new_socket(sock, new_sock);
-					TAILQ_INSERT_TAIL(env->sockets,
+					TAILQ_INSERT_TAIL(&env->sockets,
 					    new_sock, entry);
 					continue;
 				}
@@ -284,7 +284,7 @@ sockets_launch(void)
 {
 	struct socket *sock;
 
-	TAILQ_FOREACH(sock, gotwebd_env->sockets, entry) {
+	TAILQ_FOREACH(sock, &gotwebd_env->sockets, entry) {
 		log_debug("%s: configuring socket %d (%d)", __func__,
 		    sock->conf.id, sock->fd);
 
@@ -307,7 +307,7 @@ sockets_purge(struct gotwebd *env)
 	struct socket *sock, *tsock;
 
 	/* shutdown and remove sockets */
-	TAILQ_FOREACH_SAFE(sock, env->sockets, entry, tsock) {
+	TAILQ_FOREACH_SAFE(sock, &env->sockets, entry, tsock) {
 		if (event_initialized(&sock->ev))
 			event_del(&sock->ev);
 		if (evtimer_initialized(&sock->evt))
@@ -316,7 +316,7 @@ sockets_purge(struct gotwebd *env)
 			evtimer_del(&sock->pause);
 		if (sock->fd != -1)
 			close(sock->fd);
-		TAILQ_REMOVE(env->sockets, sock, entry);
+		TAILQ_REMOVE(&env->sockets, sock, entry);
 	}
 }
 
@@ -394,18 +394,16 @@ sockets_shutdown(void)
 	sockets_purge(gotwebd_env);
 
 	/* clean sockets */
-	TAILQ_FOREACH_SAFE(sock, gotwebd_env->sockets, entry, tsock) {
-		TAILQ_REMOVE(gotwebd_env->sockets, sock, entry);
+	TAILQ_FOREACH_SAFE(sock, &gotwebd_env->sockets, entry, tsock) {
+		TAILQ_REMOVE(&gotwebd_env->sockets, sock, entry);
 		close(sock->fd);
 		free(sock);
 	}
 
 	/* clean servers */
-	TAILQ_FOREACH_SAFE(srv, gotwebd_env->servers, entry, tsrv)
+	TAILQ_FOREACH_SAFE(srv, &gotwebd_env->servers, entry, tsrv)
 		free(srv);
 
-	free(gotwebd_env->sockets);
-	free(gotwebd_env->servers);
 	free(gotwebd_env);
 }
 
@@ -447,7 +445,7 @@ sockets_unix_socket_listen(struct privsep *ps, struct 
 	int u_fd = -1;
 	mode_t old_umask, mode;
 
-	TAILQ_FOREACH(tsock, env->sockets, entry) {
+	TAILQ_FOREACH(tsock, &env->sockets, entry) {
 		if (strcmp(tsock->conf.unix_socket_name,
 		    sock->conf.unix_socket_name) == 0 &&
 		    tsock->fd != -1)