From: Theo Buehler <tb@theobuehler.org>
Subject: Re: rpki-client: add rsync baseuri-based batching
To: Job Snijders <job@bsd.nl>, tech@openbsd.org
Date: Tue, 7 Jul 2026 20:37:22 +0200

On Tue, Jul 07, 2026 at 08:06:40PM +0200, Claudio Jeker wrote:
> On Tue, Jul 07, 2026 at 03:41:15PM +0000, Job Snijders wrote:
> > On Tue, Jul 07, 2026 at 12:09:12PM +0000, Job Snijders wrote:
> > > Question: I'm not sure the newly added calls to rsync_base_uri()
> > > could ever fail, because nca_history_load() will already have called
> > > rsync_base_uri() via valid_uri() on those same C strings, so perhaps
> > > instead of 'continue' I should use an errx()? Or something else?
> > 
> > Here is a better version: no duplicate calls to rsync_base_uri() and
> > more symmetry with rpkiNotify batching. Feedback? OK?
> > 
> > Index: extern.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> > diff -u -p -r1.287 extern.h
> > --- extern.h	1 Jul 2026 11:09:12 -0000	1.287
> > +++ extern.h	7 Jul 2026 15:40:47 -0000
> > @@ -159,6 +159,7 @@ struct nca_hist {
> >  	char			*ski;
> >  	char			*location;
> >  	char			*mfturi;
> > +	char			*baseuri;

This member needs freeing in nca_hist_free()

[...]

> > +static int
> > +find_in_batchlist(const char *uri)
> > +{
> > +	struct fqdnlistentry *fle;
> > +	size_t uri_len;
> > +
> > +	uri_len = strlen(uri);
> > +	LIST_FOREACH(fle, &batchlist, entry) {
> > +		if (strlen(fle->fqdn) == uri_len &&
> > +		    strncasecmp(uri, fle->fqdn, uri_len) == 0) {
> > +			return 1;
> > +		}
> > +	}
> 
> Why not simply use:
> 	LIST_FOREACH(fle, &batchlist, entry) {
> 		if (strcasecmp(uri, fle->fqdn,fle->fqdn) == 0)
> 			return 1;
> 	}
> 	return 0;
> 
> I don't understand why the strlen are needed here.

It's not needed here since we're dealing with full strings.

This comes from the code here being a copy of a copy of the skiplist and
shortlist dance in main.c, where the length check is needed since we
compare against a substring of the host.

If we're going to introduce such helpers, I think it would be worthwhile
to try covering skiplist and shortlist as well. It would then probably
be worth considering a fle->fqdn_len member.