On Fri, Aug 21, 2020 at 10:17:16AM +0200, Theo Buehler wrote:
> On Fri, Aug 21, 2020 at 10:08:50AM +0200, Jan Klemkow wrote:
> > Hi,
> >
> > The following diff simplifies a switch case statement with no functional
> > change.
> >
> > In case of IMSG_CTL_SHOW_DATABASE duplicated code is removed. This part
> > of the code is now the same as in ospfd.
>
> This part looks fine.
>
> >
> > In case of IMSG_CTL_SHOW_DB_INTRA the code was also duplicated and due
> > to a missing continue the v->type variable was double check.
>
> This doesn't look correct. v->type was checked against
> LSA_TYPE_INTRA_A_PREFIX which is not the same as
> LSA_TYPE_INTER_A_PREFIX
>
> I can't tell if a continue; is missing (which looks likely) in the
> IMSG_CTL_SHOW_DB_INTRA case or if the check against both INTRA and INTER
> is deliberate.
Good catch. Yes the continue is missing in that case.
The IMSG_CTL_SHOW_DB_INTRA case should only send back LSA with type
LSA_TYPE_INTRA_A_PREFIX. LSA_TYPE_INTER_A_PREFIX should not be included
there. This is one of the extra LSA types ospfv3 added to the mix.
> >
> > OK?
> >
> > bye,
> > Jan
> >
> > Index: rde_lsdb.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospf6d/rde_lsdb.c,v
> > retrieving revision 1.43
> > diff -u -p -r1.43 rde_lsdb.c
> > --- rde_lsdb.c 17 Feb 2020 08:12:22 -0000 1.43
> > +++ rde_lsdb.c 21 Aug 2020 07:52:15 -0000
> > @@ -763,9 +763,7 @@ lsa_dump(struct lsa_tree *tree, int imsg
> > lsa_age(v);
> > switch (imsg_type) {
> > case IMSG_CTL_SHOW_DATABASE:
> > - rde_imsg_compose_ospfe(IMSG_CTL_SHOW_DATABASE, 0, pid,
> > - &v->lsa->hdr, ntohs(v->lsa->hdr.len));
> > - continue;
> > + break;
> > case IMSG_CTL_SHOW_DB_SELF:
> > if (v->lsa->hdr.adv_rtr == rde_router_id())
> > break;
> > @@ -787,8 +785,6 @@ lsa_dump(struct lsa_tree *tree, int imsg
> > break;
> > continue;
> > case IMSG_CTL_SHOW_DB_INTRA:
> > - if (v->type == LSA_TYPE_INTRA_A_PREFIX)
> > - break;
> > case IMSG_CTL_SHOW_DB_SUM:
> > if (v->type == LSA_TYPE_INTER_A_PREFIX)
> > break;
> >
>
--
:wq Claudio
