On Sat, Apr 15, 2023 at 11:21:52PM +0800, James Raphael Tiovalen wrote:
> This commit adds null pointer checks in some code that performs some
> decisions based on old and new input ovsdb_rows. The corresponding
> non-null assertion and early return are added if both the old and the
> new ovsdb_rows are NULL.
>
> Signed-off-by: James Raphael Tiovalen <[email protected]>
...
> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
> index 4a0d6512f..34f5e041f 100644
> --- a/ovsdb/monitor.c
> +++ b/ovsdb/monitor.c
> @@ -1330,8 +1330,23 @@ ovsdb_monitor_changes_update(const struct ovsdb_row
> *old,
> const struct ovsdb_monitor_table *mt,
> struct ovsdb_monitor_change_set_for_table *mcst)
> {
> - const struct uuid *uuid = ovsdb_row_get_uuid(new ? new : old);
> - struct ovsdb_monitor_row *change;
> + const struct uuid *uuid = NULL;
> +
> + if (!new && !old) {
> + return;
> + } else {
> + if (new) {
> + uuid = ovsdb_row_get_uuid(new);
> + } else if (old) {
> + uuid = ovsdb_row_get_uuid(old);
> + }
> + }
> +
> + if (!uuid) {
> + return;
> + }
This seems a bit redundant. Could we go for something like the following?
const struct uuid *uuid = NULL;
if (new) {
uuid = ovsdb_row_get_uuid(new);
} else if (old) {
uuid = ovsdb_row_get_uuid(old);
}
if (!uuid) {
return;
}
> + struct ovsdb_monitor_row *change = NULL;
I'd slightly prefer if this was at the top of the function.
>
> change = ovsdb_monitor_changes_row_find(mcst, uuid);
> if (!change) {
> --
> 2.40.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
