I suspected this was a Static Analysis change. I'd prefer (b), but don't care enough really to complain if I got a (c) as well.
donald On Tue, Jun 7, 2016 at 1:26 PM, Christian Franke < [email protected]> wrote: > On 06/07/2016 04:10 AM, Donald Sharp wrote: > > Christian - > > > > How is this possible? In zfpm_encode_route we set cmd == RTM_DELROUTE > > if rib == NULL. > > Yes you are right, this seems not to be an issue in practice, I misread > what zfpm_encode_route does, yesterday. > > The change came up from an issue found by coverity that boils down to > this check that I removed: > > > - if (rib) > > - ri->rtm_protocol = netlink_proto_from_route_type (rib->type); > > Coverity infers from the existance of the check that rib might be NULL > at that point and complains about rib being dereferenced later on, > without any guard for NULL: > > > if ((rib->flags & ZEBRA_FLAG_BLACKHOLE) || (rib->flags & > > ZEBRA_FLAG_REJECT)) > > discard = 1; > > Given that we never have rib == NULL and cmd != RTM_DELROUTE the way > that it's currently called, I see the following options: > > a) Leave it as is and mark the coverity issue as non-problematic and > live with the code that is a bit misleading to read > b) Remove the if (rib) guard > c) Remove it and add an assert(rib != NULL) > > I am fine with either and would tend to go with c), but I am open to > suggestions. > > -Christian > >
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
