From: Theo Buehler <tb@theobuehler.org>
Subject: Re: relayd: fix GCC16 warnings
To: Rafael Sadowski <rafael@sizeofvoid.org>
Cc: tech@openbsd.org
Date: Sat, 24 Oct 2026 08:55:27 +0200

On Fri, Oct 23, 2026 at 09:58:19PM +0200, Rafael Sadowski wrote:
> Make GCC16 happy with relayd code
> 
> ca.c:348:50: warning: operand of '?:' changes signedness from 'int' to 'unsigned int' due to unsignedness of other operand [-Wsign-compare]
>   348 |                 cko.cko_tlen = (ret > 0) ? len : -1;
> 
> config.c:1219:36: warning: 'cert' may be used uninitialized [-Wmaybe-uninitialized]
>  1219 |                 cert->cert_ocsp_fd = imsg_get_fd(imsg);
>       |                 ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
> config.c:1186:34: note: 'cert' was declared here
>  1186 |         struct relay_cert       *cert;
>       |                                  ^~~~

Maybe we should just ignore these warnings... Adding kludges to badly
written code to appease a dumb compiler isn't making the world better
place.

> diff --git a/ca.c b/ca.c
> index 4bd3e72..39d1254 100644
> --- a/ca.c
> +++ b/ca.c
> @@ -345,7 +345,7 @@ ca_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
>  		ret = ECDSA_sign(0, ibuf_data(&ibuf), ibuf_size(&ibuf), to,
>  		    &len, ecdsa);
>  
> -		cko.cko_tlen = (ret > 0) ? len : -1;
> +		cko.cko_tlen = (ret > 0) ? (int)len : -1;

Elsewhere there's patterns similar to this:

		cko.cko_tlen = -1;
		iov[c].iov_base = &cko;
		iov[c++].iov_len = sizeof(cko);
		if (ECDSA_sign(...)) {
			cko.cko_tlen = len;
			iof[c].iov_base = to;
			iov[c++].iov_len = len;
		}


>  		iov[c].iov_base = &cko;
>  		iov[c++].iov_len = sizeof(cko);
>  		if (ret > 0) {
> diff --git a/config.c b/config.c
> index d1f8f8c..073c877 100644
> --- a/config.c
> +++ b/config.c
> @@ -1183,7 +1183,7 @@ config_getrelayfd(struct relayd *env, struct imsg *imsg)
>  {
>  	struct ctl_relayfd	 crfd;
>  	struct relay		*rlay = NULL;
> -	struct relay_cert	*cert;
> +	struct relay_cert	*cert = NULL;

This makes no sense to me. If the warning flagged an actual problem,
you'd change from dereferencing a bad pointer to dereferencing a NULL
pointer, which is hardly any better.

I think the second switch should be split into two switches and merged
into the first one. Then even gcc should see that there is no use of
uninitialized here.

	switch (crfd.type) {
	case RELAY_FD_CERT:
	case RELAY_FD_KEY:
	case RELAY_FD_OCSP:
		// find cert then set appropriate fd
		switch (crfd.type) {
		case RELAY_FD_CERT:
			cert->cert_fd = imsg_get_fd(imsg);
			break;
		...
		}
		/* FALLTHROUGH */
	defalut:
		// find rlay and then set appropriate fd
		switch (crfd.type) {
		case RELAY_FD_CACERT:
			rlay->rl_tls_ca_fd = imsg_get_fd(imsg);
			break;
		...
	}
	
>  
>  	if (imsg_get_data(imsg, &crfd, sizeof(crfd)) == -1) {
>  		log_warn("%s: imsg_get_data", __func__);
>