On Wed, Mar 31, 2021 at 12:40:57PM +0200, Claudio Jeker wrote:
> The current code to cleanup the repository after validation did not
> cleanup directories and also skipped any repo directory that is no
> referenced.
>
> Adjust the cleanup code to fix these two issues. This uses the fact that
> the cache directory now only contains 2 directories rsync & ta.
> Thanks to the internal order of the RB tree, the code can use RB_NFIND()
> to figure out if a path is used or not.
>
> With this the cleanup works but is also more aggressiv.
> For example if you run
> rpki-client -t /etc/rpki/afrinic.tal
> then this will clean out all other repositories from arin, apnic, lacnic
> and ripe. For regular use this is no isssue since you normally run
> rpki-client over and over again with the same flags.
>
This version also skips the cleanup run if rpki-client was started with -n
--
:wq Claudio
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.59
diff -u -p -r1.59 extern.h
--- extern.h 29 Mar 2021 12:41:34 -0000 1.59
+++ extern.h 31 Mar 2021 08:38:33 -0000
@@ -306,6 +306,7 @@ struct stats {
size_t vrps; /* total number of vrps */
size_t uniqs; /* number of unique vrps */
size_t del_files; /* number of files removed in cleanup */
+ size_t del_dirs ;/* number of directories removed in cleanup */
char *talnames;
struct timeval elapsed_time;
struct timeval user_time;
Index: main.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.126
diff -u -p -r1.126 main.c
--- main.c 29 Mar 2021 03:45:35 -0000 1.126
+++ main.c 31 Mar 2021 11:39:56 -0000
@@ -154,6 +154,22 @@ filepath_exists(char *file)
return RB_FIND(filepath_tree, &fpt, &needle) != NULL;
}
+/*
+ * Return true if a filepath entry exists that starts with path.
+ */
+static int
+filepath_dir_exists(char *path)
+{
+ struct filepath needle;
+ struct filepath *res;
+
+ needle.file = path;
+ res = RB_NFIND(filepath_tree, &fpt, &needle);
+ if (res != NULL && strstr(res->file, path) == res->file)
+ return 1;
+ return 0;
+}
+
RB_GENERATE(filepath_tree, filepath, entry, filepathcmp);
void
@@ -767,56 +783,56 @@ add_to_del(char **del, size_t *dsz, char
return del;
}
-static size_t
+static void
repo_cleanup(void)
{
- size_t i, delsz = 0;
- char *argv[2], **del = NULL;
- struct repo *rp;
+ size_t i, delsz = 0, dirsz = 0;
+ char *argv[3], **del = NULL, **dir = NULL;
FTS *fts;
FTSENT *e;
- SLIST_FOREACH(rp, &repos, entry) {
- argv[0] = rp->local;
- argv[1] = NULL;
- if ((fts = fts_open(argv, FTS_PHYSICAL | FTS_NOSTAT,
- NULL)) == NULL)
- err(1, "fts_open");
- errno = 0;
- while ((e = fts_read(fts)) != NULL) {
- switch (e->fts_info) {
- case FTS_NSOK:
- if (!filepath_exists(e->fts_path))
- del = add_to_del(del, &delsz,
- e->fts_path);
- break;
- case FTS_D:
- case FTS_DP:
- /* TODO empty directory pruning */
- break;
- case FTS_SL:
- case FTS_SLNONE:
- warnx("symlink %s", e->fts_path);
- del = add_to_del(del, &delsz, e->fts_path);
- break;
- case FTS_NS:
- case FTS_ERR:
- warnx("fts_read %s: %s", e->fts_path,
- strerror(e->fts_errno));
- break;
- default:
- warnx("unhandled[%x] %s", e->fts_info,
+ argv[0] = "ta";
+ argv[1] = "rsync";
+ argv[2] = NULL;
+ if ((fts = fts_open(argv, FTS_PHYSICAL | FTS_NOSTAT, NULL)) == NULL)
+ err(1, "fts_open");
+ errno = 0;
+ while ((e = fts_read(fts)) != NULL) {
+ switch (e->fts_info) {
+ case FTS_NSOK:
+ if (!filepath_exists(e->fts_path))
+ del = add_to_del(del, &delsz,
e->fts_path);
- break;
- }
-
- errno = 0;
+ break;
+ case FTS_D:
+ break;
+ case FTS_DP:
+ if (!filepath_dir_exists(e->fts_path))
+ dir = add_to_del(dir, &dirsz,
+ e->fts_path);
+ break;
+ case FTS_SL:
+ case FTS_SLNONE:
+ warnx("symlink %s", e->fts_path);
+ del = add_to_del(del, &delsz, e->fts_path);
+ break;
+ case FTS_NS:
+ case FTS_ERR:
+ warnx("fts_read %s: %s", e->fts_path,
+ strerror(e->fts_errno));
+ break;
+ default:
+ warnx("unhandled[%x] %s", e->fts_info,
+ e->fts_path);
+ break;
}
- if (errno)
- err(1, "fts_read");
- if (fts_close(fts) == -1)
- err(1, "fts_close");
+
+ errno = 0;
}
+ if (errno)
+ err(1, "fts_read");
+ if (fts_close(fts) == -1)
+ err(1, "fts_close");
for (i = 0; i < delsz; i++) {
if (unlink(del[i]) == -1)
@@ -826,8 +842,17 @@ repo_cleanup(void)
free(del[i]);
}
free(del);
+ stats.del_files = delsz;
- return delsz;
+ 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]);
+ free(dir[i]);
+ }
+ free(dir);
+ stats.del_dirs = dirsz;
}
void
@@ -1228,7 +1253,8 @@ main(int argc, char *argv[])
}
}
- stats.del_files = repo_cleanup();
+ if (!noop)
+ repo_cleanup();
gettimeofday(&now_time, NULL);
timersub(&now_time, &start_time, &stats.elapsed_time);
@@ -1259,7 +1285,8 @@ main(int argc, char *argv[])
logx("Certificate revocation lists: %zu", stats.crls);
logx("Ghostbuster records: %zu", stats.gbrs);
logx("Repositories: %zu", stats.repos);
- logx("Files removed: %zu", stats.del_files);
+ logx("Cleanup: removed %zu files, %zu directories",
+ stats.del_files, stats.del_dirs);
logx("VRP Entries: %zu (%zu unique)", stats.vrps, stats.uniqs);
/* Memory cleanup. */
