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