On 5/29/26 8:24 PM, Mike Pattrick via dev wrote: > On Thu, May 21, 2026 at 11:23 AM Timothy Redaelli <[email protected]> > wrote: > >> On Wed, 20 May 2026 13:49:57 -0400 >> Mike Pattrick <[email protected]> wrote: >> >>> On Tue, May 19, 2026 at 9:20 AM Timothy Redaelli via dev < >>> [email protected]> wrote: >>> >>>> The condition (flow->actions || flow->actions_len) allows >>>> flow->actions to be NULL when flow->actions_len is nonzero, >>>> which would pass NULL to nl_msg_put_unspec(). >>>> >>>> Simplify the guard to just check flow->actions, since a non-NULL >>>> actions pointer is the meaningful condition for serialization, and >>>> nl_msg_put_unspec already handles actions_len == 0 correctly. >>>> >>>> Found by OpenScanHub Coverity (FORWARD_NULL). >>>> Signed-off-by: Timothy Redaelli <[email protected]> >>>> --- >>>> lib/dpif-netlink.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c >>>> index f22a87934..7bd9449a8 100644 >>>> --- a/lib/dpif-netlink.c >>>> +++ b/lib/dpif-netlink.c >>>> @@ -4712,7 +4712,7 @@ dpif_netlink_flow_to_ofpbuf(const struct >>>> dpif_netlink_flow *flow, >>>> put_exclude_packet_type(buf, OVS_FLOW_ATTR_MASK, >> flow->mask, >>>> flow->mask_len); >>>> } >>>> - if (flow->actions || flow->actions_len) { >>>> >>> >>> Should this be "flow->actions && flow->actions_len" instead? >> >> Maybe yes, but this changes the current behavior and I preferred to >> only fix warns without changing pre-existing stuff. >> >> actions_len == 0 with a non-NULL pointer is valid; it means "drop" >> (empty actions list). >> >> See the comment right above the struct member: >> >> "If 'actions' is nonnull then OVS_FLOW_ATTR_ACTIONS will be included >> in the Netlink version of the command, even if actions_len is zero." >> >> dpif_netlink_init_flow_put() relies on this: it sets a static >> dummy_action with actions_len 0 so that OVS_FLOW_ATTR_ACTIONS is >> always serialized. Using && would skip that and break SET operations >> that need to clear actions. >> >> nl_msg_put_unspec() already handles size 0 fine (via nullable_memcpy). >> > > Ok, that's reasonable > > Acked-by: Mike Pattrick <[email protected]>
Thanks, Timothy and Mike! Applied. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
