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]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
