From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: rpki-client: p is a dumb name for a cert
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Thu, 29 Jan 2026 10:25:47 +0100

On Thu, Jan 29, 2026 at 09:11:45AM +0100, Theo Buehler wrote:
> This is one of the many remnants of struct parse that have annoyed me
> for too long. There are two separate commits in this diff:
> 
> 1. in ta_validate() and cert_free() do a simple s/\<p\>/cert/g.
> 
> 2. in cert_buffer() and cert_read() prepare a purely mechanical
>    replacement step for these two functions that doesn't result in
>    overlong lines.
> 
> I left out the third commit that does the actual replacement in
> cert_buffer() and cert_read() because the resulting diff isn't
> very readable. Not sure it makes much sense to send that out.

Thanks for splittin this up in small chunks. This is a trivial diff.
OK claudio@
 
> Index: cert.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> diff -u -p -r1.220 cert.c
> --- cert.c	28 Jan 2026 08:42:07 -0000	1.220
> +++ cert.c	29 Jan 2026 07:57:34 -0000
> @@ -2050,27 +2050,27 @@ ta_check_validity(const char *fn, struct
>   * Returns cert passed in on success or NULL on failure.
>   */
>  struct cert *
> -ta_validate(const char *fn, struct cert *p, const unsigned char *spki,
> +ta_validate(const char *fn, struct cert *cert, const unsigned char *spki,
>      size_t spkisz)
>  {
> -	if (p == NULL)
> +	if (cert == NULL)
>  		return NULL;
>  
> -	if (p->purpose != CERT_PURPOSE_TA) {
> +	if (cert->purpose != CERT_PURPOSE_TA) {
>  		warnx("%s: expected trust anchor purpose, got %s", fn,
> -		    purpose2str(p->purpose));
> +		    purpose2str(cert->purpose));
>  		goto out;
>  	}
>  
> -	if (!ta_check_pubkey(fn, p, spki, spkisz))
> +	if (!ta_check_pubkey(fn, cert, spki, spkisz))
>  		goto out;
> -	if (!ta_check_validity(fn, p))
> +	if (!ta_check_validity(fn, cert))
>  		goto out;
>  
> -	return p;
> +	return cert;
>  
>   out:
> -	cert_free(p);
> +	cert_free(cert);
>  	return NULL;
>  }
>  
> @@ -2100,25 +2100,25 @@ cert_parse_ta(const char *fn, const unsi
>   * Passing NULL is a noop.
>   */
>  void
> -cert_free(struct cert *p)
> +cert_free(struct cert *cert)
>  {
> -	if (p == NULL)
> +	if (cert == NULL)
>  		return;
>  
> -	free(p->crl);
> -	free(p->repo);
> -	free(p->path);
> -	free(p->mft);
> -	free(p->notify);
> -	free(p->signedobj);
> -	free(p->ips);
> -	free(p->ases);
> -	free(p->aia);
> -	free(p->aki);
> -	free(p->ski);
> -	free(p->pubkey);
> -	X509_free(p->x509);
> -	free(p);
> +	free(cert->crl);
> +	free(cert->repo);
> +	free(cert->path);
> +	free(cert->mft);
> +	free(cert->notify);
> +	free(cert->signedobj);
> +	free(cert->ips);
> +	free(cert->ases);
> +	free(cert->aia);
> +	free(cert->aki);
> +	free(cert->ski);
> +	free(cert->pubkey);
> +	X509_free(cert->x509);
> +	free(cert);
>  }
>  
>  /*
> @@ -2165,7 +2165,8 @@ cert_buffer(struct ibuf *b, const struct
>  		io_str_buffer(b, p->ski);
>  		io_str_buffer(b, p->pubkey);
>  	} else {
> -		errx(1, "%s: unexpected %s", __func__, purpose2str(p->purpose));
> +		errx(1, "%s: unexpected %s", __func__,
> +		    purpose2str(p->purpose));
>  	}
>  }
>  
> @@ -2191,15 +2192,19 @@ cert_read(struct ibuf *b)
>  	io_read_buf(b, &p->num_ases, sizeof(p->num_ases));
>  
>  	if (p->num_ips > 0) {
> -		if ((p->ips = calloc(p->num_ips, sizeof(p->ips[0]))) == NULL)
> +		p->ips = calloc(p->num_ips, sizeof(p->ips[0]));
> +		if (p->ips == NULL)
>  			err(1, NULL);
> -		io_read_buf(b, p->ips, p->num_ips * sizeof(p->ips[0]));
> +		io_read_buf(b, p->ips,
> +		    p->num_ips * sizeof(p->ips[0]));
>  	}
>  
>  	if (p->num_ases > 0) {
> -		if ((p->ases = calloc(p->num_ases, sizeof(p->ases[0]))) == NULL)
> +		p->ases = calloc(p->num_ases, sizeof(p->ases[0]));
> +		if (p->ases == NULL)
>  			err(1, NULL);
> -		io_read_buf(b, p->ases, p->num_ases * sizeof(p->ases[0]));
> +		io_read_buf(b, p->ases,
> +		    p->num_ases * sizeof(p->ases[0]));
>  	}
>  
>  	io_read_str(b, &p->path);
> @@ -2228,7 +2233,8 @@ cert_read(struct ibuf *b)
>  		io_read_str(b, &p->ski);
>  		io_read_str(b, &p->pubkey);
>  	} else {
> -		errx(1, "%s: unexpected %s", __func__, purpose2str(p->purpose));
> +		errx(1, "%s: unexpected %s", __func__,
> +		    purpose2str(p->purpose));
>  	}
>  
>  	return p;
> 

-- 
:wq Claudio