From: Stefan Sperling Subject: Re: gotwebd address list head allocation To: "Todd C. Miller" Cc: gameoftrees@openbsd.org Date: Tue, 16 Aug 2022 16:50:04 +0200 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)