On 28 Aug 2025, at 13:07, Mike Pattrick wrote:
> 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.
Thanks, yes I prefer this also over assert() will change it in my v2.
> -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
