On 11/3/22 09:47, Roi Dayan wrote:
> Remove the redundant null pointer assignment and move
> the features reset from wrapper caller to the get features
> function to be consistent that dpif wrappers just call
> the dpif class implemented callback and not doing
> anything else.
>
> CC: Justin Pettit <[email protected]>
> Signed-off-by: Roi Dayan <[email protected]>
> ---
>
> Notes:
> v2:
> - move memset from wrapper call
>
> lib/dpif-netlink.c | 2 +-
> lib/dpif.c | 1 -
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index a620a6ec52dd..2bdd2137af36 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -4105,7 +4105,7 @@ dpif_netlink_meter_get_features(const struct dpif
> *dpif_,
> struct ofputil_meter_features *features)
> {
> if (probe_broken_meters(CONST_CAST(struct dpif *, dpif_))) {
> - features = NULL;
> + memset(features, 0, sizeof *features);> return;
> }
>
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 40f5fe44606e..f00aeea37428 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1935,7 +1935,6 @@ void
> dpif_meter_get_features(const struct dpif *dpif,
> struct ofputil_meter_features *features)
> {
> - memset(features, 0, sizeof *features);
I'm not sure this is correct, because we should still clear them
even if the dpif_class doesn't have the callback.
All providers seem to implement this callback, but not clearing
features doesn't seem correct from the API clarity point of view.
Just returning without clearing the structure in
dpif_netlink_meter_get_features() might be a better option.
Clearing in both places is also fine as it's not a performance
critical code path.
> if (dpif->dpif_class->meter_get_features) {
> dpif->dpif_class->meter_get_features(dpif, features);
> }
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
