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.
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;
