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

Reply via email to