On Thu, Aug 28, 2025 at 3:39 AM Eelco Chaudron <echau...@redhat.com> wrote:

>
>
> On 27 Aug 2025, at 20:57, Mike Pattrick wrote:
>
> > On Tue, Aug 26, 2025 at 8:35 AM Eelco Chaudron via dev <
> > ovs-dev@openvswitch.org> wrote:
> >
> >> Fixing this Coverity error by adding an ovs_assert() for the
> >> case not allowed.
> >>
> >> Signed-off-by: Eelco Chaudron <echau...@redhat.com>
> >> ---
> >>  ovsdb/monitor.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
> >> index c3bfae3d2..8b1923e29 100644
> >> --- a/ovsdb/monitor.c
> >> +++ b/ovsdb/monitor.c
> >> @@ -1022,6 +1022,7 @@ ovsdb_monitor_compose_row_update(
> >>                                                  &c->column->type));
> >>          }
> >>          if (type & (OJMS_INITIAL | OJMS_INSERT | OJMS_MODIFY)) {
> >> +            ovs_assert(row->new);
> >>
> >
> > This isn't bad persay, but couldn't we just change the logic in
> > ovsdb_monitor_row_update_type?
>
> Hi Mike,
>
> I guess we can do it there. We might even change the return to OJMS_NONE
> instead of the assert.
>
> --- a/ovsdb/monitor.c
> +++ b/ovsdb/monitor.c
> @@ -681,6 +681,12 @@ ovsdb_monitor_change_set_destroy(struct
> ovsdb_monitor_change_set *mcs)
>  static enum ovsdb_monitor_selection
>  ovsdb_monitor_row_update_type(bool initial, const bool old, const bool
> new)
>  {
> +    ovs_assert(initial && !new);
> +
>      return initial ? OJMS_INITIAL
>              : !old ? OJMS_INSERT
>              : !new ? OJMS_DELETE
>
>
> or
>
> @@ -681,7 +681,9 @@ ovsdb_monitor_change_set_destroy(struct
> ovsdb_monitor_change_set *mcs)
>  static enum ovsdb_monitor_selection
>  ovsdb_monitor_row_update_type(bool initial, const bool old, const bool
> new)
>  {
> -    return initial ? OJMS_INITIAL
> +    return initial ? (new ? OJMS_INITIAL : OJMS_NONE)
>              : !old ? OJMS_INSERT
>              : !new ? OJMS_DELETE
>              : OJMS_MODIFY;
>
>
If you return OJMS_NONE here then ovsdb_monitor_compose_row_update() will
cause the function to just return early, which may be preferable to an
assert.

-M



> Let me know what you think/prefer, and I’ll change it in a V2.
>
> //Eelco
>
> > -M
> >
> >
> >>              json_object_put(new_json, c->column->name,
> >>                              ovsdb_datum_to_json(&row->new[i],
> >>                                                  &c->column->type));
> >> --
> >> 2.50.1
> >>
> >> _______________________________________________
> >> dev mailing list
> >> d...@openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>
> >>
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to