On Tue, Nov 11, 2025 at 11:05 AM Ales Musil via dev <[email protected]> wrote:
> 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. > My bad. Missed it. Thanks. Acked-by: Numan Siddique <[email protected]> Numan > > > > 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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
