On 7/11/23 12:17, Eelco Chaudron wrote:
>
>
> On 13 Jun 2023, at 20:34, James Raphael Tiovalen wrote:
>
>> This commit adds non-null pointer assertions in some code that performs
>> some decisions based on old and new input ovsdb_rows.
>>
>> Signed-off-by: James Raphael Tiovalen <[email protected]>
>> Reviewed-by: Simon Horman <[email protected]>
>
> What about error messages/argument checking instead of asserts here?
File transactions must have some data. Every row must have at least
uuid and version. New rows should have 'new' versions of the data,
deleted should have 'old', and modified should have both. It's a
bug somewhere if we have a row without any data.
>
> //Eelco
>
>> ---
>> ovsdb/file.c | 2 ++
>> ovsdb/monitor.c | 4 +++-
>> 2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/ovsdb/file.c b/ovsdb/file.c
>> index 2d887e53e..b1d386e76 100644
>> --- a/ovsdb/file.c
>> +++ b/ovsdb/file.c
>> @@ -522,7 +522,9 @@ ovsdb_file_txn_add_row(struct ovsdb_file_txn *ftxn,
>> }
>>
>> if (row) {
>> + ovs_assert(new || old);
>> struct ovsdb_table *table = new ? new->table : old->table;
>> + ovs_assert(table);
>> char uuid[UUID_LEN + 1];
>>
>> if (table != ftxn->table) {
>> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
>> index 4afaa89f4..c32af7b02 100644
>> --- a/ovsdb/monitor.c
>> +++ b/ovsdb/monitor.c
>> @@ -1372,8 +1372,10 @@ ovsdb_monitor_changes_update(const struct ovsdb_row
>> *old,
>> const struct ovsdb_monitor_table *mt,
>> struct ovsdb_monitor_change_set_for_table
>> *mcst)
>> {
>> + ovs_assert(new || old);
>> const struct uuid *uuid = ovsdb_row_get_uuid(new ? new : old);
>> - struct ovsdb_monitor_row *change;
>> + ovs_assert(uuid);
>> + struct ovsdb_monitor_row *change = NULL;
>>
>> change = ovsdb_monitor_changes_row_find(mcst, uuid);
>> if (!change) {
>> --
>> 2.40.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev