Re: rpki-client proc_parser cleanup

[Theo Buehler]

On Mon, Oct 25, 2021 at 07:50:46PM +0200, Claudio Jeker wrote:
> Refactor this code and instead of passing various things around just use
> globals.

ok with two small nits:

> @@ -560,14 +622,6 @@ proc_parser(int fd)
>   if ((pfd.revents & POLLHUP))
>   break;
>  
> - /*
> -  * Start with read events.
> -  * This means that the parent process is sending us
> -  * something we need to parse.
> -  * We don't actually parse it til we have space in our
> -  * outgoing buffer for responding, though.
> -  */
> -
>   if ((pfd.revents & POLLIN)) {
>   b = io_buf_read(fd, &inbuf);
>   

Could you zap the two tabs on the line after io_buf_read() while you're here?

[...]
> + parse_entity(&q, &msgq);
> + 
>   }

No empty line before the closing braces (or at least nuke the three tabs)

rpki-client proc_parser cleanup

[Claudio Jeker]

Refactor this code and instead of passing various things around just use
globals.

-- 
:wq Claudio

Index: parser.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
retrieving revision 1.16
diff -u -p -r1.16 parser.c
--- parser.c23 Oct 2021 20:01:16 -  1.16
+++ parser.c25 Oct 2021 16:55:11 -
@@ -38,20 +38,23 @@
 #include "extern.h"
 
 static void build_chain(const struct auth *, STACK_OF(X509) **);
-static struct crl  *get_crl(const struct auth *, struct crl_tree *);
+static struct crl  *get_crl(const struct auth *);
 static void build_crls(const struct crl *, STACK_OF(X509_CRL) **);
 
 /* Limit how deep the RPKI tree can be. */
 #defineMAX_CERT_DEPTH  12
 
+static X509_STORE_CTX  *ctx;
+static struct auth_tree  auths = RB_INITIALIZER(&auths);
+static struct crl_tree  crlt = RB_INITIALIZER(&crlt);
+
 /*
  * Parse and validate a ROA.
  * This is standard stuff.
  * Returns the roa on success, NULL on failure.
  */
 static struct roa *
-proc_parser_roa(struct entity *entp, X509_STORE_CTX *ctx,
-struct auth_tree *auths, struct crl_tree *crlt)
+proc_parser_roa(struct entity *entp)
 {
struct roa  *roa;
X509*x509;
@@ -64,10 +67,10 @@ proc_parser_roa(struct entity *entp, X50
if ((roa = roa_parse(&x509, entp->file)) == NULL)
return NULL;
 
-   a = valid_ski_aki(entp->file, auths, roa->ski, roa->aki);
+   a = valid_ski_aki(entp->file, &auths, roa->ski, roa->aki);
 
build_chain(a, &chain);
-   crl = get_crl(a, crlt);
+   crl = get_crl(a);
build_crls(crl, &crls);
 
assert(x509 != NULL);
@@ -113,7 +116,7 @@ proc_parser_roa(struct entity *entp, X50
 * the code around roa_read() to check the "valid" field itself.
 */
 
-   if (valid_roa(entp->file, auths, roa))
+   if (valid_roa(entp->file, &auths, roa))
roa->valid = 1;
 
sk_X509_free(chain);
@@ -134,8 +137,7 @@ proc_parser_roa(struct entity *entp, X50
  * Return the mft on success or NULL on failure.
  */
 static struct mft *
-proc_parser_mft(struct entity *entp, X509_STORE_CTX *ctx,
-   struct auth_tree *auths, struct crl_tree *crlt)
+proc_parser_mft(struct entity *entp)
 {
struct mft  *mft;
X509*x509;
@@ -146,7 +148,7 @@ proc_parser_mft(struct entity *entp, X50
if ((mft = mft_parse(&x509, entp->file)) == NULL)
return NULL;
 
-   a = valid_ski_aki(entp->file, auths, mft->ski, mft->aki);
+   a = valid_ski_aki(entp->file, &auths, mft->ski, mft->aki);
build_chain(a, &chain);
 
if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL))
@@ -187,8 +189,7 @@ proc_parser_mft(struct entity *entp, X50
  * parse failure.
  */
 static struct cert *
-proc_parser_cert(const struct entity *entp, X509_STORE_CTX *ctx,
-struct auth_tree *auths, struct crl_tree *crlt)
+proc_parser_cert(const struct entity *entp)
 {
struct cert *cert;
X509*x509;
@@ -205,9 +206,9 @@ proc_parser_cert(const struct entity *en
if (cert == NULL)
return NULL;
 
-   a = valid_ski_aki(entp->file, auths, cert->ski, cert->aki);
+   a = valid_ski_aki(entp->file, &auths, cert->ski, cert->aki);
build_chain(a, &chain);
-   build_crls(get_crl(a, crlt), &crls);
+   build_crls(get_crl(a), &crls);
 
assert(x509 != NULL);
if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL))
@@ -236,7 +237,7 @@ proc_parser_cert(const struct entity *en
sk_X509_CRL_free(crls);
 
/* Validate the cert to get the parent */
-   if (!valid_cert(entp->file, auths, cert)) {
+   if (!valid_cert(entp->file, &auths, cert)) {
X509_free(x509); // needed? XXX
return cert;
}
@@ -262,7 +263,7 @@ proc_parser_cert(const struct entity *en
if (na->fn == NULL)
err(1, NULL);
 
-   if (RB_INSERT(auth_tree, auths, na) != NULL)
+   if (RB_INSERT(auth_tree, &auths, na) != NULL)
err(1, "auth tree corrupted");
 
return cert;
@@ -279,8 +280,7 @@ proc_parser_cert(const struct entity *en
  * parse failure.
  */
 static struct cert *
-proc_parser_root_cert(const struct entity *entp, X509_STORE_CTX *ctx,
-struct auth_tree *auths, struct crl_tree *crlt)
+proc_parser_root_cert(const struct entity *entp)
 {
charsubject[256];
ASN1_TIME   *notBefore, *notAfter;
@@ -327,7 +327,7 @@ proc_parser_root_cert(const struct entit
subject);
goto badcert;
}
-   if (!valid_ta(entp->file, auths, cert)) {
+   if (!valid_ta(entp->file, &auths, cert)) {
warnx("%s: certificate not a valid ta, subject='%s'",
entp->file, subject);
goto badce