On Mon, Jan 04, 2021 at 12:23:35PM +0100, Claudio Jeker wrote:
> On Mon, Jan 04, 2021 at 12:09:46PM +0100, Theo Buehler wrote:
> > Pointed out by llvm scan-build. mrt_config is much larger (> 10x). As
> > far as I can tell, this isn't bad. It just overallocates and copies a
> > lot of zeroes thanks to the calloc() in parse.y.
> > 
> > Perhaps it would be better to use sizeof(*xm) instead.
> 
> I think this is wrong. There is a difference between struct mrt_config and mrt
> but the code uses MRT2MC() in various places to change between the types.
> This code could use some work to clean up this mess.

Oh good, so this is intentional. Thanks.

>  
> > Regress passes with the Makefile diff at the end (is there a better
> > way?).
> 
> Regress diff is OK. This was the addition of 'bgpctl show sets' that added
> the need for getmonotime().
>  
> > Index: usr.sbin/bgpd/mrt.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/mrt.c,v
> > retrieving revision 1.103
> > diff -u -p -r1.103 mrt.c
> > --- usr.sbin/bgpd/mrt.c     9 Jan 2020 11:55:25 -0000       1.103
> > +++ usr.sbin/bgpd/mrt.c     4 Jan 2021 10:16:18 -0000
> > @@ -1031,9 +1031,9 @@ mrt_mergeconfig(struct mrt_head *xconf, 
> >     LIST_FOREACH(m, nconf, entry) {
> >             if ((xm = mrt_get(xconf, m)) == NULL) {
> >                     /* NEW */
> > -                   if ((xm = malloc(sizeof(struct mrt_config))) == NULL)
> > +                   if ((xm = malloc(sizeof(struct mrt))) == NULL)
> >                             fatal("mrt_mergeconfig");
> > -                   memcpy(xm, m, sizeof(struct mrt_config));
> > +                   memcpy(xm, m, sizeof(struct mrt));
> >                     xm->state = MRT_STATE_OPEN;
> >                     LIST_INSERT_HEAD(xconf, xm, entry);
> >             } else {
> > Index: usr.sbin/bgpd/parse.y
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> > retrieving revision 1.411
> > diff -u -p -r1.411 parse.y
> > --- usr.sbin/bgpd/parse.y   29 Dec 2020 15:30:34 -0000      1.411
> > +++ usr.sbin/bgpd/parse.y   4 Jan 2021 10:17:26 -0000
> > @@ -3871,7 +3871,7 @@ add_mrtconfig(enum mrt_type type, char *
> >             }
> >     }
> >  
> > -   if ((n = calloc(1, sizeof(struct mrt_config))) == NULL)
> > +   if ((n = calloc(1, sizeof(struct mrt))) == NULL)
> >             fatal("add_mrtconfig");
> >  
> >     n->type = type;
> > Index: regress/usr.sbin/bgpd/unittests/Makefile
> > ===================================================================
> > RCS file: /cvs/src/regress/usr.sbin/bgpd/unittests/Makefile,v
> > retrieving revision 1.6
> > diff -u -p -r1.6 Makefile
> > --- regress/usr.sbin/bgpd/unittests/Makefile        29 Dec 2020 16:57:50 
> > -0000      1.6
> > +++ regress/usr.sbin/bgpd/unittests/Makefile        4 Jan 2021 10:37:38 
> > -0000
> > @@ -14,11 +14,11 @@ CFLAGS+= -I${.CURDIR} -I${.CURDIR}/../..
> >  LDADD= -lutil
> >  DPADD+= ${LIBUTIL}
> >  
> > -SRCS_rde_sets_test=        rde_sets_test.c rde_sets.c
> > +SRCS_rde_sets_test=        rde_sets_test.c rde_sets.c timer.c log.c
> >  run-regress-rde_sets_test: rde_sets_test
> >     ./rde_sets_test
> >  
> > -SRCS_rde_trie_test=        rde_trie_test.c rde_trie.c util.c rde_sets.c
> > +SRCS_rde_trie_test=        rde_trie_test.c rde_trie.c util.c rde_sets.c 
> > timer.c log.c
> >  TRIE_TESTS=1 2 3 4 5 6
> >  TRIE4_FLAGS=-o
> >  TRIE5_FLAGS=-r
> > 
> 
> -- 
> :wq Claudio

Reply via email to