> On Nov 3, 2022, at 3:38 PM, Ilya Maximets <[email protected]> wrote:
>
> On 11/3/22 09:47, Roi Dayan wrote:
>>
>> 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.
Agreed. This is the point I was trying to make in my original feedback: I
think the patch can just remove the "features = NULL;" line in
dpif_netlink_meter_get_features() and leave everything else as-is. So the
patch essentially becomes, since dpif_meter_get_features() already cleared out
'features':
-=-=-=-=-=-=-=-
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -4105,7 +4105,6 @@ 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;
return;
}
-=-=-=-=-=-=-=-
--Justin
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
