On Tue, Apr 25, 2023 at 4:12 AM Ilya Maximets <[email protected]> wrote:
>
> Every table must have a _uuid column, it's internal and should always be 
> there.
> So, I'd replace this with just an assertion.
>

Sure, I will change this to an assertion.

>
> This changes behavior of ovsdb-server when user is sending cond_change
> request without previously setting any conditions, i.e. the monitor v1
> is in use.  Documentation doesn't say what should be done in this case.
> Right now the server will return a success reply without doing anything.
> It's arguable if it is good or bad, but a) we probably need a better
> error message for the user, b) its unclear why that would be flagged by
> Coverity as an issue as it doesn't look like there could be any potential
> NULL dereference of something like that.
>

Hmm okay. I will undo this change.

>
> Same here.  '_version' is an internal column that should always be there.
>

Got it.

> >
> > The unwind logic below seems to already exist in this function.
> > So rather than duplicating it, could it be moved to a goto label?
> >

Sure.

>
> 'columns' is not an optional member.  Parsing should fail if it
> is not present, i.e. we should not be able to get to this line
> if columns is NULL.  It looks strange that we need to check the
> pointers that parser did already check.
>

Got it. I will also undo this part of the patch.

Best regards,
James Raphael Tiovalen
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to