On Mon, Sep 25, 2023 at 05:02:06PM +0200, Theo Buehler wrote: > On Mon, Sep 25, 2023 at 04:43:31PM +0200, Claudio Jeker wrote: > > On Mon, Sep 25, 2023 at 04:38:48PM +0200, Theo Buehler wrote: > > > On Mon, Sep 25, 2023 at 02:47:37PM +0200, Claudio Jeker wrote: > > > > On Sat, Sep 23, 2023 at 01:23:34PM +0200, Theo Buehler wrote: > > > > > This is a second chunk split out of the diff mentioned in my previous > > > > > mail. It factors the parsing of ASIdentifiers and IPAddrBlocks out of > > > > > sbgp_assysnum() and sbgp_ipaddrblk() and makes the latter only extract > > > > > the info from the X509_EXTENSION. This should not change anything, but > > > > > the logic is a bit tricky. > > > > > > > > > > We could initialize *as and *asz, as well as *ips and *ipsz to NULL/0, > > > > > at the top of the two new sbgp_parse_*. > > > > > > > > It looks inded like nthing is changed. The thing I dislike a bit is how > > > > **as and *asz are updated inside the sbgp_parse_* functions. There is > > > > return 0 before and after the calloc / recallocarray calls and so it > > > > depends a lot on the caller to be careful here. The code right now is > > > > ok. > > > > > > Thanks for that clue. I didn't particularly like my diff either. The > > > below is better, has less churn and should be easier to review. This way > > > the caller doesn't have to be careful. > > > > > > I left the currently existing variables asz and ipsz untouched since it > > > becomes too confusing. I want to rename asz -> sz and new_asz -> asz in > > > a follow-up, similarly for ipsz. > > > > Indeed much better. OK claudio@ > > And here's the rename. It is mechanical apart from two lines where I > fixed the order of the variable declarations and from a line I > unwrapped.
OK claudio@ > Index: cert.c > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v > retrieving revision 1.116 > diff -u -p -r1.116 cert.c > --- cert.c 25 Sep 2023 14:56:20 -0000 1.116 > +++ cert.c 25 Sep 2023 14:58:57 -0000 > @@ -159,7 +159,7 @@ sbgp_parse_assysnum(const char *fn, cons > { > const ASIdOrRanges *aors = NULL; > struct cert_as *as = NULL; > - size_t asz, new_asz = 0; > + size_t asz = 0, sz; > int i; > > assert(*out_as == NULL && *out_asz == 0); > @@ -178,11 +178,11 @@ sbgp_parse_assysnum(const char *fn, cons > > switch (asidentifiers->asnum->type) { > case ASIdentifierChoice_inherit: > - asz = 1; > + sz = 1; > break; > case ASIdentifierChoice_asIdsOrRanges: > aors = asidentifiers->asnum->u.asIdsOrRanges; > - asz = sk_ASIdOrRange_num(aors); > + sz = sk_ASIdOrRange_num(aors); > break; > default: > warnx("%s: RFC 3779 section 3.2.3.2: ASIdentifierChoice: " > @@ -190,21 +190,21 @@ sbgp_parse_assysnum(const char *fn, cons > goto out; > } > > - if (asz == 0) { > + if (sz == 0) { > warnx("%s: RFC 6487 section 4.8.11: empty asIdsOrRanges", fn); > goto out; > } > - if (asz >= MAX_AS_SIZE) { > + if (sz >= MAX_AS_SIZE) { > warnx("%s: too many AS number entries: limit %d", > fn, MAX_AS_SIZE); > goto out; > } > - as = calloc(asz, sizeof(struct cert_as)); > + as = calloc(sz, sizeof(struct cert_as)); > if (as == NULL) > err(1, NULL); > > if (aors == NULL) { > - if (!sbgp_as_inherit(fn, as, &new_asz)) > + if (!sbgp_as_inherit(fn, as, &asz)) > goto out; > } > > @@ -214,11 +214,11 @@ sbgp_parse_assysnum(const char *fn, cons > aor = sk_ASIdOrRange_value(aors, i); > switch (aor->type) { > case ASIdOrRange_id: > - if (!sbgp_as_id(fn, as, &new_asz, aor->u.id)) > + if (!sbgp_as_id(fn, as, &asz, aor->u.id)) > goto out; > break; > case ASIdOrRange_range: > - if (!sbgp_as_range(fn, as, &new_asz, aor->u.range)) > + if (!sbgp_as_range(fn, as, &asz, aor->u.range)) > goto out; > break; > default: > @@ -229,7 +229,7 @@ sbgp_parse_assysnum(const char *fn, cons > } > > *out_as = as; > - *out_asz = new_asz; > + *out_asz = asz; > > return 1; > > @@ -361,7 +361,7 @@ sbgp_parse_ipaddrblk(const char *fn, con > const IPAddressOrRange *aor; > enum afi afi; > struct cert_ip *ips = NULL; > - size_t ipsz, new_ipsz = 0; > + size_t ipsz = 0, sz; > int i, j; > > assert(*out_ips == NULL && *out_ipsz == 0); > @@ -372,27 +372,26 @@ sbgp_parse_ipaddrblk(const char *fn, con > switch (af->ipAddressChoice->type) { > case IPAddressChoice_inherit: > aors = NULL; > - ipsz = new_ipsz + 1; > + sz = ipsz + 1; > break; > case IPAddressChoice_addressesOrRanges: > aors = af->ipAddressChoice->u.addressesOrRanges; > - ipsz = new_ipsz + sk_IPAddressOrRange_num(aors); > + sz = ipsz + sk_IPAddressOrRange_num(aors); > break; > default: > warnx("%s: RFC 3779: IPAddressChoice: unknown type %d", > fn, af->ipAddressChoice->type); > goto out; > } > - if (ipsz == new_ipsz) { > + if (sz == ipsz) { > warnx("%s: RFC 6487 section 4.8.10: " > "empty ipAddressesOrRanges", fn); > goto out; > } > > - if (ipsz >= MAX_IP_SIZE) > + if (sz >= MAX_IP_SIZE) > goto out; > - ips = recallocarray(ips, new_ipsz, ipsz, > - sizeof(struct cert_ip)); > + ips = recallocarray(ips, ipsz, sz, sizeof(struct cert_ip)); > if (ips == NULL) > err(1, NULL); > > @@ -402,7 +401,7 @@ sbgp_parse_ipaddrblk(const char *fn, con > } > > if (aors == NULL) { > - if (!sbgp_addr_inherit(fn, ips, &new_ipsz, afi)) > + if (!sbgp_addr_inherit(fn, ips, &ipsz, afi)) > goto out; > continue; > } > @@ -411,12 +410,12 @@ sbgp_parse_ipaddrblk(const char *fn, con > aor = sk_IPAddressOrRange_value(aors, j); > switch (aor->type) { > case IPAddressOrRange_addressPrefix: > - if (!sbgp_addr(fn, ips, &new_ipsz, afi, > + if (!sbgp_addr(fn, ips, &ipsz, afi, > aor->u.addressPrefix)) > goto out; > break; > case IPAddressOrRange_addressRange: > - if (!sbgp_addr_range(fn, ips, &new_ipsz, afi, > + if (!sbgp_addr_range(fn, ips, &ipsz, afi, > aor->u.addressRange)) > goto out; > break; > @@ -429,7 +428,7 @@ sbgp_parse_ipaddrblk(const char *fn, con > } > > *out_ips = ips; > - *out_ipsz = new_ipsz; > + *out_ipsz = ipsz; > > return 1; > > -- :wq Claudio