From 512ef3b56ec5cd26f8ccfa15cb7599da43f51518 Mon Sep 17 00:00:00 2001
From: claudio <claudio@openbsd.org>
Date: Tue, 4 May 2021 08:16:36 +0000
Subject: [PATCH] Similar to rsync repo delay deletes for rrdp repos to after
 processing all files. Now RRDP has a weak sync mechanism and therefor not all
 unreferenced files can be removed, instead look at the list of removes which
 were part of the delta processing and remove those files. Warn and keep the
 file if a delta specifies to remove a file which is actually still
 referenced. OK benno@

---
 usr.sbin/rpki-client/repo.c | 92 ++++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 37 deletions(-)

diff --git a/usr.sbin/rpki-client/repo.c b/usr.sbin/rpki-client/repo.c
index 8f40e491d29..8448ca06063 100644
--- a/usr.sbin/rpki-client/repo.c
+++ b/usr.sbin/rpki-client/repo.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: repo.c,v 1.6 2021/04/19 17:04:35 deraadt Exp $ */
+/*	$OpenBSD: repo.c,v 1.7 2021/05/04 08:16:36 claudio Exp $ */
 /*
  * Copyright (c) 2021 Claudio Jeker <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -553,16 +553,19 @@ rrdp_free(void)
 	}
 }
 
-static int
+static struct rrdprepo *
 rrdp_basedir(const char *dir)
 {
 	struct rrdprepo *rr;
 
 	SLIST_FOREACH(rr, &rrdprepos, entry)
-		if (strcmp(dir, rr->basedir) == 0)
-			return 1;
+		if (strcmp(dir, rr->basedir) == 0) {
+			if (rr->state == REPO_FAILED)
+				return NULL;
+			return rr;
+		}
 
-	return 0;
+	return NULL;
 }
 
 /*
@@ -840,27 +843,6 @@ rrdp_merge_repo(struct rrdprepo *rr)
 	struct filepath *fp, *nfp;
 	char *fn, *rfn;
 
-	/* XXX should delay deletes */
-	RB_FOREACH_SAFE(fp, filepath_tree, &rr->deleted, nfp) {
-		fn = rrdp_filename(rr, fp->file, 1);
-		rfn = rrdp_filename(rr, fp->file, 0);
-
-		if (fn == NULL || rfn == NULL)
-			errx(1, "bad filepath");	/* should not happen */
-
-		if (unlink(rfn) == -1) {
-			if (errno == ENOENT) {
-				if (unlink(fn) == -1)
-					warn("%s: unlink", fn);
-			} else
-				warn("%s: unlink", rfn);
-		}
-
-		free(rfn);
-		free(fn);
-		filepath_put(&rr->deleted, fp);
-	}
-
 	RB_FOREACH_SAFE(fp, filepath_tree, &rr->added, nfp) {
 		fn = rrdp_filename(rr, fp->file, 1);
 		rfn = rrdp_filename(rr, fp->file, 0);
@@ -1146,12 +1128,40 @@ add_to_del(char **del, size_t *dsz, char *file)
 	*dsz = i + 1;
 	return del;
 }
+
+static char **
+repo_rrdp_cleanup(struct filepath_tree *tree, struct rrdprepo *rr,
+    char **del, size_t *delsz)
+{
+	struct filepath *fp, *nfp;
+	char *fn;
+
+	RB_FOREACH_SAFE(fp, filepath_tree, &rr->deleted, nfp) {
+		fn = rrdp_filename(rr, fp->file, 0);
+		/* temp dir will be cleaned up by repo_cleanup() */
+
+		if (fn == NULL)
+			errx(1, "bad filepath");	/* should not happen */
+
+		if (!filepath_exists(tree, fn))
+			del = add_to_del(del, delsz, fn);
+		else
+			warnx("%s: referenced file supposed to be deleted", fn);
+
+		free(fn);
+		filepath_put(&rr->deleted, fp);
+	}
+
+	return del;
+}
+
 void
 repo_cleanup(struct filepath_tree *tree)
 {
-	size_t i, delsz = 0, dirsz = 0;
+	size_t i, cnt, delsz = 0, dirsz = 0;
 	char **del = NULL, **dir = NULL;
 	char *argv[4] = { "ta", "rsync", "rrdp", NULL };
+	struct rrdprepo *rr;
 	FTS *fts;
 	FTSENT *e;
 
@@ -1166,10 +1176,12 @@ repo_cleanup(struct filepath_tree *tree)
 				    e->fts_path);
 			break;
 		case FTS_D:
-			/* skip rrdp base directories during cleanup */
-			if (rrdp_basedir(e->fts_path))
+			/* special cleanup for rrdp directories */
+			if ((rr = rrdp_basedir(e->fts_path)) != NULL) {
+				del = repo_rrdp_cleanup(tree, rr, del, &delsz);
 				if (fts_set(fts, e, FTS_SKIP) == -1)
 					err(1, "fts_set");
+			}
 			break;
 		case FTS_DP:
 			if (!filepath_dir_exists(tree, e->fts_path))
@@ -1203,25 +1215,31 @@ repo_cleanup(struct filepath_tree *tree)
 	if (fts_close(fts) == -1)
 		err(1, "fts_close");
 
+	cnt = 0;
 	for (i = 0; i < delsz; i++) {
-		if (unlink(del[i]) == -1)
-			warn("unlink %s", del[i]);
-		if (verbose > 1)
-			logx("deleted %s", del[i]);
+		if (unlink(del[i]) == -1) {
+			if (errno != ENOENT)
+				warn("unlink %s", del[i]);
+		} else {
+			if (verbose > 1)
+				logx("deleted %s", del[i]);
+			cnt++;
+		}
 		free(del[i]);
 	}
 	free(del);
-	stats.del_files = delsz;
+	stats.del_files = cnt;
 
+	cnt = 0;
 	for (i = 0; i < dirsz; i++) {
 		if (rmdir(dir[i]) == -1)
 			warn("rmdir %s", dir[i]);
-		if (verbose > 1)
-			logx("deleted dir %s", dir[i]);
+		else
+			cnt++;
 		free(dir[i]);
 	}
 	free(dir);
-	stats.del_dirs = dirsz;
+	stats.del_dirs = cnt;
 }
 
 void
-- 
2.20.1