Re: rpki-client: add rsync baseuri-based batching

Job Snijders <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]> 
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 <[email protected]>
+ *
+ * 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);
+	}
+}
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.