From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: add log_roa() and log_aspa()
To: tech@openbsd.org
Date: Wed, 10 Jan 2024 13:02:54 +0100

On Wed, Jan 10, 2024 at 12:05:54PM +0100, Claudio Jeker wrote:
> Extract the code to print a roa or aspa entry from printconf and convert
> it into log_roa() and log_aspa().
> 
> Now log_aspa() is a bit complex because the size of the buffer is not
> known. This now mallocs a buffer large enough and then tries to reuse that
> buffer. I hope I did not screw this up too badly.
> 
> A follow up diff will use these log functions in the rtr code.

By definition of bikeshedding, simpler diffs generate more of it. Feel
free to ignore.

ok tb

> +const char *
> +log_aspa(struct aspa_set *aspa)
> +{
> +	static char *buf;
> +	static size_t len;
> +	char asbuf[16];
> +	size_t needed;
> +	uint32_t i;
> +
> +	/* include enough space for header and trailer */
> +	if ((uint64_t)aspa->num > (SIZE_MAX / 12 - 72))
> +		goto fail;
> +	needed = (uint64_t)aspa->num * 12 + 72;

The 12 is a bit strange. How about using sizeof(asbuf)? Then you don't
need to cast twice.

> +	if (needed > len) {
> +		free(buf);
> +		len = needed;
> +		if ((buf = malloc(len)) == NULL)

not really nicer, but maybe use realloc?

> +			goto fail;
> +	}
> +
> +	snprintf(buf, len, "customer-as %s%s  provider-as { ",

is the double space before provider-as intentional?

> +	    log_as(aspa->as), log_expires(aspa->expires));
> +
> +	for (i = 0; i < aspa->num; i++) {
> +		snprintf(asbuf, sizeof(asbuf), "%s ", log_as(aspa->tas[i]));
> +		if (strlcat(buf, asbuf, len) >= len)
> +			goto fail;
> +	}
> +	if (strlcat(buf, "}", len) >= len)
> +		goto fail;
> +	return buf;
> +
> + fail:
> +	{
> +		static char errbuf[256];

I'd be inclined to make this have function scope. Then you could
unindent and without the double space unwrap the snprintf.

> +
> +		free(buf);
> +		buf = NULL;
> +		len = 0;
> +		snprintf(errbuf, sizeof(errbuf),
> +		    "customer-as %s%s  provider-as { ... }",
> +		    log_as(aspa->as), log_expires(aspa->expires));
> +		return errbuf;
> +	}
> +}
> +
>  const char *
>  log_rtr_error(enum rtr_error err)
>  {
>