Index | Thread | Search

From:
Job Snijders <job@bsd.nl>
Subject:
Re: rpki-client: add rsync baseuri-based batching
To:
Theo Buehler <tb@theobuehler.org>
Cc:
tech@openbsd.org
Date:
Wed, 8 Jul 2026 09:52:44 +0000

Download raw body.

Thread
On Tue, Jul 07, 2026 at 08:37:22PM +0200, Theo Buehler wrote:
> > 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.

OK, to table 'rsync base uri batching' for a moment...

Here is a refactor that introduces helpers for shortlist & skiplist. I
don't think a 'len' member is needed if the needles used for search are
right-sized.

Thoughts?

Index: Makefile
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/Makefile,v
diff -u -p -r1.41 Makefile
--- Makefile	22 Jun 2026 21:25:44 -0000	1.41
+++ Makefile	8 Jul 2026 09:48:25 -0000
@@ -42,6 +42,7 @@ SRCS+=	rsync.c
 SRCS+=	spl.c
 SRCS+=	tak.c
 SRCS+=	tal.c
+SRCS+=	util.c
 SRCS+=	validate.c
 SRCS+=	x509.c
 
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	8 Jul 2026 09:48:25 -0000
@@ -29,11 +29,15 @@
 
 #define MAX_MSG_SIZE	(50 * 1024 * 1024)
 
-struct fqdnlistentry {
-	LIST_ENTRY(fqdnlistentry)	entry;
-	char				*fqdn;
+struct strlistentry {
+	LIST_ENTRY(strlistentry) entry;
+	char *str;
 };
-LIST_HEAD(fqdns, fqdnlistentry);
+LIST_HEAD(strlist_head, strlistentry);
+
+void strlist_insert(struct strlist_head *, const char *);
+int strlist_find(const struct strlist_head *, const char *);
+void strlist_free(struct strlist_head *);
 
 enum cert_as_type {
 	CERT_AS_ID, /* single identifier */
Index: main.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
diff -u -p -r1.309 main.c
--- main.c	24 Jun 2026 09:06:20 -0000	1.309
+++ main.c	8 Jul 2026 09:48:26 -0000
@@ -85,8 +85,8 @@ int64_t  evaluation_time = X509_TIME_MIN
 
 struct stats	 stats;
 
-struct fqdns shortlist = LIST_HEAD_INITIALIZER(fqdns);
-struct fqdns skiplist = LIST_HEAD_INITIALIZER(fqdns);
+static struct strlist_head shortlist = LIST_HEAD_INITIALIZER(shortlist);
+static struct strlist_head skiplist = LIST_HEAD_INITIALIZER(skiplist);
 
 /*
  * Log a message to stderr if and only if "verbose" is non-zero.
@@ -509,32 +509,27 @@ static void
 queue_add_from_cert(const struct cert *cert, struct nca_tree *ncas)
 {
 	struct repo		*repo;
-	struct fqdnlistentry	*le;
-	char			*nfile, *npath, *host;
+	char			*fqdn, *nfile, *npath;
 	const char		*uri, *repouri, *file;
-	size_t			 hostsz, repourisz;
+	size_t			 repourisz;
 	int			 shortlisted = 0;
 
 	if (strncmp(cert->repo, RSYNC_PROTO, RSYNC_PROTO_LEN) != 0)
 		errx(1, "unexpected protocol");
-	host = cert->repo + RSYNC_PROTO_LEN;
-	hostsz = strcspn(host, "/");
 
-	LIST_FOREACH(le, &skiplist, entry) {
-		if (strlen(le->fqdn) == hostsz &&
-		    strncasecmp(host, le->fqdn, hostsz) == 0) {
-			warnx("skipping %s (listed in skiplist)", cert->repo);
-			return;
-		}
-	}
+	if ((fqdn = strdup(cert->repo + RSYNC_PROTO_LEN)) == NULL)
+		err(1, NULL);
+	fqdn[strcspn(fqdn, "/")] = '\0';
 
-	LIST_FOREACH(le, &shortlist, entry) {
-		if (strlen(le->fqdn) == hostsz &&
-		    strncasecmp(host, le->fqdn, hostsz) == 0) {
-			shortlisted = 1;
-			break;
-		}
+	shortlisted = strlist_find(&shortlist, fqdn);
+
+	if (strlist_find(&skiplist, fqdn)) {
+		warnx("skipping %s (listed in skiplist)", cert->repo);
+		free(fqdn);
+		return;
 	}
+	free(fqdn);
+
 	if (shortlistmode && shortlisted == 0) {
 		if (verbose)
 			warnx("skipping %s (not shortlisted)", cert->repo);
@@ -881,7 +876,6 @@ tal_load_default(void)
 static void
 load_skiplist(const char *slf)
 {
-	struct fqdnlistentry	*le;
 	FILE			*fp;
 	char			*line = NULL;
 	size_t			 linesize = 0, linelen;
@@ -910,12 +904,8 @@ load_skiplist(const char *slf)
 		if (!valid_uri(line, linelen, NULL))
 			errx(1, "invalid entry in skiplist: %s", line);
 
-		if ((le = malloc(sizeof(struct fqdnlistentry))) == NULL)
-			err(1, NULL);
-		if ((le->fqdn = strdup(line)) == NULL)
-			err(1, NULL);
+		strlist_insert(&skiplist, line);
 
-		LIST_INSERT_HEAD(&skiplist, le, entry);
 		stats.skiplistentries++;
 	}
 	if (ferror(fp))
@@ -931,18 +921,11 @@ load_skiplist(const char *slf)
 static void
 load_shortlist(const char *fqdn)
 {
-	struct fqdnlistentry	*le;
-
 	if (!valid_uri(fqdn, strlen(fqdn), NULL))
 		errx(1, "invalid fqdn passed to -q: %s", fqdn);
 
-	if ((le = malloc(sizeof(struct fqdnlistentry))) == NULL)
-		err(1, NULL);
-
-	if ((le->fqdn = strdup(fqdn)) == NULL)
-		err(1, NULL);
+	strlist_insert(&shortlist, fqdn);
 
-	LIST_INSERT_HEAD(&shortlist, le, entry);
 }
 
 static void
Index: nca.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/nca.c,v
diff -u -p -r1.8 nca.c
--- nca.c	8 Jul 2026 05:33:28 -0000	1.8
+++ nca.c	8 Jul 2026 09:48:26 -0000
@@ -123,7 +123,7 @@ certidcmp(const struct nonfunc_ca *a, co
 
 RB_GENERATE(nca_tree, nonfunc_ca, entry, certidcmp);
 
-static LIST_HEAD(, fqdnlistentry) notifys = LIST_HEAD_INITIALIZER(notifys);
+static struct strlist_head batchlist = LIST_HEAD_INITIALIZER(batchlist);
 
 static RB_HEAD(nca_hist_tree, nca_hist) ncas_hist = RB_INITIALIZER(&ncas_hist);
 
@@ -229,8 +229,6 @@ static void
 ncas_plan_retries(void)
 {
 	struct nca_hist *nca_hist;
-	struct fqdnlistentry *fle, *fle_tmp;
-	size_t notify_len;
 
 	RB_FOREACH(nca_hist, nca_hist_tree, &ncas_hist) {
 		if (nca_decide_retry(nca_hist) == 0) {
@@ -241,36 +239,20 @@ ncas_plan_retries(void)
 		if (nca_hist->notify == NULL)
 			continue;
 
-		if ((fle = malloc(sizeof(*fle))) == NULL)
-			err(1, NULL);
-
-		if ((fle->fqdn = strdup(nca_hist->notify)) == NULL)
-			err(1, NULL);
-
-		LIST_INSERT_HEAD(&notifys, fle, entry);
+		strlist_insert(&batchlist, nca_hist->notify);
 	}
 
 	RB_FOREACH(nca_hist, nca_hist_tree, &ncas_hist) {
 		if (nca_hist->notify == NULL)
 			continue;
 
-		notify_len = strlen(nca_hist->notify);
-
-		LIST_FOREACH(fle, &notifys, entry) {
-			if (strlen(fle->fqdn) == notify_len &&
-			    strncasecmp(nca_hist->notify, fle->fqdn,
-			    notify_len) == 0) {
-				nca_hist->defer = 0;
-				break;
-			}
+		if (strlist_find(&batchlist, nca_hist->notify)) {
+			nca_hist->defer = 0;
+			break;
 		}
 	}
 
-	LIST_FOREACH_SAFE(fle, &notifys, entry, fle_tmp) {
-		LIST_REMOVE(fle, entry);
-		free(fle->fqdn);
-		free(fle);
-	}
+	strlist_free(&batchlist);
 }
 
 void
Index: util.c
===================================================================
RCS file: util.c
diff -N util.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ util.c	8 Jul 2026 09:48:26 -0000
@@ -0,0 +1,64 @@
+/*	$OpenBSD$ */
+/*
+ * Copyright (c) 2026 Job Snijders <job@bsd.nl>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <sys/queue.h>
+
+#include <err.h>
+#include <stdlib.h>
+#include <string.h>
+#include <strings.h>
+
+#include "extern.h"
+
+void
+strlist_insert(struct strlist_head *strlist, const char *str)
+{
+	struct strlistentry *sle;
+
+	if ((sle = malloc(sizeof(struct strlistentry))) == NULL)
+		err(1, NULL);
+
+	if ((sle->str = strdup(str)) == NULL)
+		err(1, NULL);
+
+	LIST_INSERT_HEAD(strlist, sle, entry);
+}
+
+int
+strlist_find(const struct strlist_head *strlist, const char *str)
+{
+	struct strlistentry *sle;
+
+	LIST_FOREACH(sle, strlist, entry) {
+		if (strcasecmp(str, sle->str) == 0)
+			return 1;
+	}
+
+	return 0;
+}
+
+void
+strlist_free(struct strlist_head *strlist)
+{
+	struct strlistentry *sle, *sle_tmp;
+
+	LIST_FOREACH_SAFE(sle, strlist, entry, sle_tmp) {
+		LIST_REMOVE(sle, entry);
+		free(sle->str);
+		free(sle);
+	}
+}