On Tue, Nov 11, 2025 at 4:56 PM Numan Siddique <[email protected]> wrote:
> On Tue, Nov 11, 2025 at 10:02 AM Ales Musil <[email protected]> wrote: > > > > As reported by coverity, the f can never be null: > > *** CID 493666: Null pointer dereferences (REVERSE_INULL) > > /br-controller/br-flow-mgr.c: 551 in br_flow_add_openflow__() > > 545 > > 546 if (active_flow_list_changed) { > > 547 ovn_flow_invalidate_all_actions(f); > > 548 } > > 549 > > 550 struct ovn_flow_action *existing_a = > > >>> CID 493666: Null pointer dereferences (REVERSE_INULL) > > >>> Null-checking "f" suggests that it may be null, but it has > already been dereferenced on all paths leading to the check. > > 551 f ? ovn_flow_get_matching_action(f, a): NULL; > > 552 bool push_flow_to_switch = true; > > 553 > > 554 if (existing_a && uuid_equals(&existing_a->flow_uuid, > flow_uuid)) { > > 555 existing_a->stale = false; > > 556 > > > > ** CID 493665: Null pointer dereferences (FORWARD_NULL) > > > > Fixes: 899991dd3e1e ("ovn-br-controller: Add bridge flow table manager.") > > Signed-off-by: Ales Musil <[email protected]> > > --- > > br-controller/br-flow-mgr.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/br-controller/br-flow-mgr.c b/br-controller/br-flow-mgr.c > > index 8f65237f2..07153da1e 100644 > > --- a/br-controller/br-flow-mgr.c > > +++ b/br-controller/br-flow-mgr.c > > @@ -547,8 +547,7 @@ br_flow_add_openflow__(struct br_flow_table > *br_ftable, bool lflow_table, > > ovn_flow_invalidate_all_actions(f); > > } > > > > - struct ovn_flow_action *existing_a = > > - f ? ovn_flow_get_matching_action(f, a): NULL; > > + struct ovn_flow_action *existing_a = > ovn_flow_get_matching_action(f, a); > > bool push_flow_to_switch = true; > > > > Thanks for addressing this. > > One question - The function ovn_flow_get_matching_action() can return > NULL - > https://github.com/ovn-org/ovn/blob/main/br-controller/br-flow-mgr.c#L906 > > Is the warning because the flow action check can never be NULL even > though the function returns NULL at the end ? > It should be strictly about the "f" cannot be NULL on that particular line. Because earlier we have "if (!f) {" which will allocate. > Numan > > > if (existing_a && uuid_equals(&existing_a->flow_uuid, flow_uuid)) { > > -- > > 2.51.0 > > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
