From: Job Snijders Subject: rpki-client: pass around a container struct instead of a long parameter list To: tech@openbsd.org Date: Tue, 8 Jul 2025 12:44:12 +0000 I think it is fine to continue to pass around 'everything' into the various output functions - even when a particular function doesn't use a one or more of the available trees; BUT, it seems tedious that every time we add or remove a feature, every output function's signature also has to change. Perhaps it is better to use a struct that contains references to the various trees and pass that 'container' around. I believe this kind of refactor will help avoid certain churn in future work. Thoughts? Index: extern.h =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v diff -u -p -r1.243 extern.h --- extern.h 30 Jun 2025 14:20:26 -0000 1.243 +++ extern.h 8 Jul 2025 12:40:46 -0000 @@ -501,6 +501,14 @@ struct brk { RB_HEAD(brk_tree, brk); RB_PROTOTYPE(brk_tree, brk, entry, brkcmp); +struct trees { + struct vrp_tree vrps; + struct brk_tree brks; + struct vap_tree vaps; + struct vsp_tree vsps; + struct nca_tree ncas; +}; + /* * A single CRL */ @@ -995,25 +1003,13 @@ extern int outformats; #define FORMAT_JSON 0x08 #define FORMAT_OMETRIC 0x10 -int outputfiles(struct vrp_tree *v, struct brk_tree *b, - struct vap_tree *, struct vsp_tree *, struct nca_tree *, - struct stats *); +int outputfiles(struct trees *, struct stats *); int outputheader(FILE *, struct stats *); -int output_bgpd(FILE *, struct vrp_tree *, struct brk_tree *, - struct vap_tree *, struct vsp_tree *, struct nca_tree *, - struct stats *); -int output_bird(FILE *, struct vrp_tree *, struct brk_tree *, - struct vap_tree *, struct vsp_tree *, struct nca_tree *, - struct stats *); -int output_csv(FILE *, struct vrp_tree *, struct brk_tree *, - struct vap_tree *, struct vsp_tree *, struct nca_tree *, - struct stats *); -int output_json(FILE *, struct vrp_tree *, struct brk_tree *, - struct vap_tree *, struct vsp_tree *, struct nca_tree *, - struct stats *); -int output_ometric(FILE *, struct vrp_tree *, struct brk_tree *, - struct vap_tree *, struct vsp_tree *, struct nca_tree *, - struct stats *); +int output_bgpd(FILE *, struct trees *, struct stats *); +int output_bird(FILE *, struct trees *, struct stats *); +int output_csv(FILE *, struct trees *, struct stats *); +int output_json(FILE *, struct trees *, struct stats *); +int output_ometric(FILE *, struct trees *, struct stats *); void logx(const char *fmt, ...) __attribute__((format(printf, 1, 2))); Index: main.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v diff -u -p -r1.284 main.c --- main.c 26 Jun 2025 06:00:32 -0000 1.284 +++ main.c 8 Jul 2025 12:40:46 -0000 @@ -561,9 +561,7 @@ queue_add_from_cert(const struct cert *c * In all cases, we gather statistics. */ static void -entity_process(struct ibuf *b, struct stats *st, struct vrp_tree *tree, - struct brk_tree *brktree, struct vap_tree *vaptree, - struct vsp_tree *vsptree, struct nca_tree *ncatree) +entity_process(struct ibuf *b, struct stats *st, struct trees *trees) { enum rtype type; struct tal *tal; @@ -620,10 +618,10 @@ entity_process(struct ibuf *b, struct st switch (cert->purpose) { case CERT_PURPOSE_TA: case CERT_PURPOSE_CA: - queue_add_from_cert(cert, ncatree); + queue_add_from_cert(cert, &trees->ncas); break; case CERT_PURPOSE_BGPSEC_ROUTER: - cert_insert_brks(brktree, cert); + cert_insert_brks(&trees->brks, cert); repo_stat_inc(rp, talid, type, STYPE_BGPSEC); break; default: @@ -642,7 +640,7 @@ entity_process(struct ibuf *b, struct st if (mft->seqnum_gap) repo_stat_inc(rp, talid, type, STYPE_SEQNUM_GAP); queue_add_from_mft(mft); - cert_remove_nca(ncatree, mft->certid, rp); + cert_remove_nca(&trees->ncas, mft->certid, rp); mft_free(mft); break; case RTYPE_CRL: @@ -657,7 +655,7 @@ entity_process(struct ibuf *b, struct st } roa = roa_read(b); if (roa->valid) - roa_insert_vrps(tree, roa, rp); + roa_insert_vrps(&trees->vrps, roa, rp); else repo_stat_inc(rp, talid, type, STYPE_INVALID); roa_free(roa); @@ -672,7 +670,7 @@ entity_process(struct ibuf *b, struct st } aspa = aspa_read(b); if (aspa->valid) - aspa_insert_vaps(file, vaptree, aspa, rp); + aspa_insert_vaps(file, &trees->vaps, aspa, rp); else repo_stat_inc(rp, talid, type, STYPE_INVALID); aspa_free(aspa); @@ -686,7 +684,7 @@ entity_process(struct ibuf *b, struct st } spl = spl_read(b); if (spl->valid) - spl_insert_vsps(vsptree, spl, rp); + spl_insert_vsps(&trees->vsps, spl, rp); else repo_stat_inc(rp, talid, type, STYPE_INVALID); spl_free(spl); @@ -989,16 +987,18 @@ main(int argc, char *argv[]) const char *cachedir = NULL, *outputdir = NULL; const char *errs, *name; const char *skiplistfile = NULL; - struct vrp_tree vrps = RB_INITIALIZER(&vrps); - struct vsp_tree vsps = RB_INITIALIZER(&vsps); - struct brk_tree brks = RB_INITIALIZER(&brks); - struct vap_tree vaps = RB_INITIALIZER(&vaps); - struct nca_tree ncas = RB_INITIALIZER(&ncas); + struct trees trees; struct rusage ru; struct timespec start_time, now_time; clock_gettime(CLOCK_MONOTONIC, &start_time); + RB_INIT(&trees.vrps); + RB_INIT(&trees.brks); + RB_INIT(&trees.vaps); + RB_INIT(&trees.vsps); + RB_INIT(&trees.ncas); + /* If started as root, priv-drop to _rpki-client */ if (getuid() == 0) { struct passwd *pw; @@ -1414,8 +1414,7 @@ main(int argc, char *argv[]) errx(1, "ibuf_read: connection closed"); } while ((b = io_buf_get(queues[0])) != NULL) { - entity_process(b, &stats, &vrps, &brks, &vaps, - &vsps, &ncas); + entity_process(b, &stats, &trees); ibuf_free(b); } } @@ -1508,7 +1507,7 @@ main(int argc, char *argv[]) } repo_stats_collect(sum_repostats, &stats.repo_stats); - if (outputfiles(&vrps, &brks, &vaps, &vsps, &ncas, &stats)) + if (outputfiles(&trees, &stats)) rc = 1; printf("Processing time %lld seconds " Index: output-bgpd.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/output-bgpd.c,v diff -u -p -r1.33 output-bgpd.c --- output-bgpd.c 27 Mar 2025 05:03:09 -0000 1.33 +++ output-bgpd.c 8 Jul 2025 12:40:46 -0000 @@ -20,9 +20,7 @@ #include "extern.h" int -output_bgpd(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks, - struct vap_tree *vaps, struct vsp_tree *vsps, struct nca_tree *ncas, - struct stats *st) +output_bgpd(FILE *out, struct trees *trees, struct stats *st) { struct vrp *vrp; struct vap *vap; @@ -34,7 +32,7 @@ output_bgpd(FILE *out, struct vrp_tree * if (fprintf(out, "roa-set {\n") < 0) return -1; - RB_FOREACH(vrp, vrp_tree, vrps) { + RB_FOREACH(vrp, vrp_tree, &trees->vrps) { char ipbuf[64], maxlenbuf[100]; ip_addr_print(&vrp->addr, vrp->afi, ipbuf, sizeof(ipbuf)); @@ -58,7 +56,7 @@ output_bgpd(FILE *out, struct vrp_tree * if (fprintf(out, "\naspa-set {\n") < 0) return -1; - RB_FOREACH(vap, vap_tree, vaps) { + RB_FOREACH(vap, vap_tree, &trees->vaps) { if (vap->overflowed) continue; if (fprintf(out, "\tcustomer-as %d expires %lld " Index: output-bird.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/output-bird.c,v diff -u -p -r1.23 output-bird.c --- output-bird.c 27 Mar 2025 05:03:09 -0000 1.23 +++ output-bird.c 8 Jul 2025 12:40:46 -0000 @@ -21,9 +21,7 @@ #include "extern.h" int -output_bird(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks, - struct vap_tree *vaps, struct vsp_tree *vsps, struct nca_tree *ncas, - struct stats *st) +output_bird(FILE *out, struct trees *trees, struct stats *st) { struct vrp *v; struct vap *vap; @@ -49,7 +47,7 @@ output_bird(FILE *out, struct vrp_tree * "\troa4 { table ROAS4; };\n\n") < 0) return -1; - RB_FOREACH(v, vrp_tree, vrps) { + RB_FOREACH(v, vrp_tree, &trees->vrps) { char buf[64]; if (v->afi == AFI_IPV4) { @@ -64,7 +62,7 @@ output_bird(FILE *out, struct vrp_tree * "\troa6 { table ROAS6; };\n\n") < 0) return -1; - RB_FOREACH(v, vrp_tree, vrps) { + RB_FOREACH(v, vrp_tree, &trees->vrps) { char buf[64]; if (v->afi == AFI_IPV6) { @@ -85,7 +83,7 @@ output_bird(FILE *out, struct vrp_tree * "};\n\n") < 0) return -1; - RB_FOREACH(vap, vap_tree, vaps) { + RB_FOREACH(vap, vap_tree, &trees->vaps) { if (vap->overflowed) continue; if (fprintf(out, "\troute aspa %d providers ", Index: output-csv.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/output-csv.c,v diff -u -p -r1.15 output-csv.c --- output-csv.c 27 Mar 2025 05:03:09 -0000 1.15 +++ output-csv.c 8 Jul 2025 12:40:46 -0000 @@ -20,16 +20,14 @@ #include "extern.h" int -output_csv(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks, - struct vap_tree *vaps, struct vsp_tree *vsps, struct nca_tree *ncas, - struct stats *st) +output_csv(FILE *out, struct trees *trees, struct stats *st) { struct vrp *v; if (fprintf(out, "ASN,IP Prefix,Max Length,Trust Anchor,Expires\n") < 0) return -1; - RB_FOREACH(v, vrp_tree, vrps) { + RB_FOREACH(v, vrp_tree, &trees->vrps) { char buf[64]; ip_addr_print(&v->addr, v->afi, buf, sizeof(buf)); Index: output-json.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v diff -u -p -r1.53 output-json.c --- output-json.c 3 Apr 2025 14:29:44 -0000 1.53 +++ output-json.c 8 Jul 2025 12:40:46 -0000 @@ -145,9 +145,7 @@ output_spl(struct vsp_tree *vsps) } int -output_json(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks, - struct vap_tree *vaps, struct vsp_tree *vsps, struct nca_tree *ncas, - struct stats *st) +output_json(FILE *out, struct trees *trees, struct stats *st) { char buf[64]; struct vrp *v; @@ -158,7 +156,7 @@ output_json(FILE *out, struct vrp_tree * outputheader_json(st); json_do_array("roas"); - RB_FOREACH(v, vrp_tree, vrps) { + RB_FOREACH(v, vrp_tree, &trees->vrps) { ip_addr_print(&v->addr, v->afi, buf, sizeof(buf)); json_do_object("roa", 1); @@ -172,7 +170,7 @@ output_json(FILE *out, struct vrp_tree * json_do_end(); json_do_array("bgpsec_keys"); - RB_FOREACH(b, brk_tree, brks) { + RB_FOREACH(b, brk_tree, &trees->brks) { json_do_object("brks", 0); json_do_int("asn", b->asid); json_do_string("ski", b->ski); @@ -184,7 +182,7 @@ output_json(FILE *out, struct vrp_tree * json_do_end(); json_do_array("nonfunc_cas"); - RB_FOREACH(nca, nca_tree, ncas) { + RB_FOREACH(nca, nca_tree, &trees->ncas) { json_do_object("nca", 1); json_do_string("location", nca->location); json_do_string("ta", taldescs[nca->talid]); @@ -196,10 +194,10 @@ output_json(FILE *out, struct vrp_tree * json_do_end(); if (!excludeaspa) - output_aspa(vaps); + output_aspa(&trees->vaps); if (experimental) - output_spl(vsps); + output_spl(&trees->vsps); return json_do_finish(); } Index: output-ometric.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/output-ometric.c,v diff -u -p -r1.14 output-ometric.c --- output-ometric.c 3 Apr 2025 14:29:44 -0000 1.14 +++ output-ometric.c 8 Jul 2025 12:40:46 -0000 @@ -167,9 +167,7 @@ repo_stats(const struct repo *rp, const } int -output_ometric(FILE *out, struct vrp_tree *vrps, struct brk_tree *brks, - struct vap_tree *vaps, struct vsp_tree *vsps, struct nca_tree *ncas, - struct stats *st) +output_ometric(FILE *out, struct trees *trees, struct stats *st) { struct olabels *ol; const char *keys[4] = { "nodename", "domainname", "release", NULL }; Index: output.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/output.c,v diff -u -p -r1.40 output.c --- output.c 3 Apr 2025 14:29:44 -0000 1.40 +++ output.c 8 Jul 2025 12:40:46 -0000 @@ -63,9 +63,7 @@ static char output_name[PATH_MAX]; static const struct outputs { int format; char *name; - int (*fn)(FILE *, struct vrp_tree *, struct brk_tree *, - struct vap_tree *, struct vsp_tree *, struct nca_tree *, - struct stats *); + int (*fn)(FILE *, struct trees *, struct stats *); } outputs[] = { { FORMAT_OPENBGPD, "openbgpd", output_bgpd }, { FORMAT_BIRD, "bird", output_bird }, @@ -124,8 +122,7 @@ prune_as0_tals(struct vrp_tree *vrps) } int -outputfiles(struct vrp_tree *v, struct brk_tree *b, struct vap_tree *a, - struct vsp_tree *p, struct nca_tree *ncas, struct stats *st) +outputfiles(struct trees *trees, struct stats *st) { int i, rc = 0; @@ -133,7 +130,7 @@ outputfiles(struct vrp_tree *v, struct b set_signal_handler(); if (excludeas0) - prune_as0_tals(v); + prune_as0_tals(&trees->vrps); for (i = 0; outputs[i].name; i++) { FILE *fout; @@ -147,7 +144,7 @@ outputfiles(struct vrp_tree *v, struct b rc = 1; continue; } - if ((*outputs[i].fn)(fout, v, b, a, p, ncas, st) != 0) { + if ((*outputs[i].fn)(fout, trees, st) != 0) { warn("output for %s format failed", outputs[i].name); fclose(fout); output_cleantmp();