On Thu, Jan 06, 2022 at 01:21:03PM +0100, Claudio Jeker wrote:
> Ran into this the other day and could not help myself to adjust the code.
> If the mft is stale just bump the stale counter and be done. If not stale
> queue all files from the mft for the next round.
>
> In mft_parse I switched to a switch statement which is more obvious in my
> opinion.
I'm ok with this.
Have you considered setting this flag directly on inspecting
check_validity()'s return value instead of bubbling rc = 0 up to
mft_parse()? This should allow us to get rid of the "Reset rc"
dance in mft_parse_econtent() and it could have a two-valued error.
> --
> :wq Claudio
>
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.171
> diff -u -p -r1.171 main.c
> --- main.c 4 Jan 2022 18:41:32 -0000 1.171
> +++ main.c 6 Jan 2022 11:41:26 -0000
> @@ -518,9 +518,10 @@ entity_process(struct ibuf *b, struct st
> break;
> }
> mft = mft_read(b);
> - if (mft->stale)
> + if (!mft->stale)
> + queue_add_from_mft_set(mft);
> + else
> st->mfts_stale++;
> - queue_add_from_mft_set(mft);
> mft_free(mft);
> break;
> case RTYPE_CRL:
> Index: mft.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 mft.c
> --- mft.c 28 Oct 2021 13:51:42 -0000 1.42
> +++ mft.c 6 Jan 2022 11:46:24 -0000
> @@ -418,8 +418,8 @@ struct mft *
> mft_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len)
> {
> struct parse p;
> - int c, rc = 0;
> - size_t i, cmsz;
> + int rc = 0;
> + size_t cmsz;
> unsigned char *cms;
>
> memset(&p, 0, sizeof(struct parse));
> @@ -451,30 +451,16 @@ mft_parse(X509 **x509, const char *fn, c
> goto out;
> }
>
> - /*
> - * If we're stale, then remove all of the files that the MFT
> - * references as well as marking it as stale.
> - */
> -
> - if ((c = mft_parse_econtent(cms, cmsz, &p)) == 0) {
> - /*
> - * FIXME: it should suffice to just mark this as stale
> - * and have the logic around mft_read() simply ignore
> - * the contents of stale entries, just like it does for
> - * invalid ROAs or certificates.
> - */
> -
> + switch (mft_parse_econtent(cms, cmsz, &p)) {
> + case 0:
> + /* Mark MFT as stale. */
> p.res->stale = 1;
> - if (p.res->files != NULL)
> - for (i = 0; i < p.res->filesz; i++)
> - free(p.res->files[i].file);
> - free(p.res->files);
> - p.res->filesz = 0;
> - p.res->files = NULL;
> - } else if (c == -1)
> + break;
> + case -1:
> goto out;
> -
> + }
> rc = 1;
> +
> out:
> if (rc == 0) {
> mft_free(p.res);
>
