Download raw body.
httpd: fix scan-build dead stores findings
On Sun, Dec 28, 2025 at 06:55:28AM +0100, Rafael Sadowski wrote:
> Hi,
>
> scan-build flagged a few dead stores in httpd:
>
> - config.c:636 – s incremented but never used after
> - httpd.c:533 – q assigned but immediately overwritten in the loop
> - server.c:891 – inrd/inwr assigned, then reassigned, never actually needed
> - server_fcgi.c:678,690 – kv result unused
>
> OK?
>
> Diff removes all of them. OK?
> 1. src/usr.sbin/httpd/config.c:636:3: warning: Value stored to 's' is never read [deadcode.DeadStores]
> 636 | s += srv_conf->return_uri_len;
>
> 2. src/usr.sbin/httpd/httpd.c:533:6: warning: Although the value stored to 'q' is used in the enclosing expression, the value is never actually read from 'q' [deadcode.DeadStores]
> 533 | p = q = label;
> | ^ ~~~~~
>
> 3. src/usr.sbin/httpd/server.c:891:15: warning: Value stored to 'inrd' during its initialization is never read [deadcode.DeadStores]
> 891 | evbuffercb inrd = server_read;
> | ^~~~ ~~~~~~~~~~~
>
> 4. src/usr.sbin/httpd/server_fcgi.c:678:9: warning: Although the value stored to 'kv' is used in the enclosing expression, the value is never actually read from 'kv' [deadcode.DeadStores]
> 678 | if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
> | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> src/usr.sbin/httpd/server_fcgi.c:690:8: warning: Although the value stored to 'kv' is used in the enclosing expression, the value is never actually read from 'kv' [deadcode.DeadStores]
> 690 | if ((kv = kv_find(&resp->http_headers, &key)) != NULL) {
> | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> diff --git a/usr.sbin/httpd/config.c b/usr.sbin/httpd/config.c
> index b45081129b7..aa2cd382ed3 100644
> --- a/usr.sbin/httpd/config.c
> +++ b/usr.sbin/httpd/config.c
> @@ -531,7 +531,6 @@ config_getserver_config(struct httpd *env, struct server *srv,
> if ((srv_conf->return_uri = get_data(p + s,
> srv_conf->return_uri_len)) == NULL)
> goto fail;
> - s += srv_conf->return_uri_len;
I am not 100% sure about this one. On one hand scan-build is right that the
value of s is never used in the remainder of this function. On the other
hand, this code seems to be written with the assumption in mind that more
optional chuks of data with variable length could be added in the future.
If we ever grow the server config structure and process more data via the
'p + s' idiom then this line would need to be added back in to avoid a bug.
So perhaps this line is worth keeping?
The rest is ok stsp@
> }
>
> if (srv_conf->flags & SRVFLAG_LOCATION) {
> diff --git a/usr.sbin/httpd/httpd.c b/usr.sbin/httpd/httpd.c
> index ab78dd9d1d1..4d20be94c2c 100644
> --- a/usr.sbin/httpd/httpd.c
> +++ b/usr.sbin/httpd/httpd.c
> @@ -530,7 +530,7 @@ expand_string(char *label, size_t len, const char *srch, const char *repl)
> log_debug("%s: calloc", __func__);
> return (-1);
> }
> - p = q = label;
> + p = label;
> while ((q = strstr(p, srch)) != NULL) {
> *q = '\0';
> if ((strlcat(tmp, p, len) >= len) ||
> diff --git a/usr.sbin/httpd/server.c b/usr.sbin/httpd/server.c
> index 5d5063b6480..709730d1f84 100644
> --- a/usr.sbin/httpd/server.c
> +++ b/usr.sbin/httpd/server.c
> @@ -887,8 +887,6 @@ void
> server_input(struct client *clt)
> {
> struct server_config *srv_conf = clt->clt_srv_conf;
> - evbuffercb inrd = server_read;
> - evbuffercb inwr = server_write;
> socklen_t slen;
>
> if (server_httpdesc_init(clt) == -1) {
> @@ -897,7 +895,6 @@ server_input(struct client *clt)
> }
>
> clt->clt_toread = TOREAD_HTTP_HEADER;
> - inrd = server_read_http;
>
> slen = sizeof(clt->clt_sndbufsiz);
> if (getsockopt(clt->clt_s, SOL_SOCKET, SO_SNDBUF,
> @@ -909,8 +906,8 @@ server_input(struct client *clt)
> /*
> * Client <-> Server
> */
> - clt->clt_bev = bufferevent_new(clt->clt_s, inrd, inwr,
> - server_error, clt);
> + clt->clt_bev = bufferevent_new(clt->clt_s, server_read_http,
> + server_write, server_error, clt);
> if (clt->clt_bev == NULL) {
> server_close(clt, "failed to allocate input buffer event");
> return;
> diff --git a/usr.sbin/httpd/server_fcgi.c b/usr.sbin/httpd/server_fcgi.c
> index f3c01a459b0..1aeb6b43ccf 100644
> --- a/usr.sbin/httpd/server_fcgi.c
> +++ b/usr.sbin/httpd/server_fcgi.c
> @@ -645,7 +645,7 @@ server_fcgi_header(struct client *clt, unsigned int code)
> struct http_descriptor *resp = clt->clt_descresp;
> const char *error;
> char tmbuf[32];
> - struct kv *kv, *cl, key;
> + struct kv *cl, key;
>
> clt->clt_fcgi.headerssent = 1;
>
> @@ -675,7 +675,7 @@ server_fcgi_header(struct client *clt, unsigned int code)
> /* But then we need a Content-Length unless method is HEAD... */
> if (desc->http_method != HTTP_METHOD_HEAD) {
> key.kv_key = "Content-Length";
> - if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
> + if (kv_find(&resp->http_headers, &key) == NULL) {
> if (kv_add(&resp->http_headers,
> "Content-Length", "0") == NULL)
> return (-1);
> @@ -687,7 +687,7 @@ server_fcgi_header(struct client *clt, unsigned int code)
> if (clt->clt_fcgi.chunked) {
> /* but only if no Content-Length header is supplied */
> key.kv_key = "Content-Length";
> - if ((kv = kv_find(&resp->http_headers, &key)) != NULL) {
> + if (kv_find(&resp->http_headers, &key) != NULL) {
> clt->clt_fcgi.chunked = 0;
> } else {
> /*
>
>
httpd: fix scan-build dead stores findings