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

Reply via email to