On Tue, Nov 11, 2025 at 5:21 PM Numan Siddique <[email protected]> wrote:
> > > 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 > > Thank you Numan, I went ahead and merged this into main. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
