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
