On 03/11/2022 7:28, Justin Pettit wrote:
>
>> On Nov 2, 2022, at 5:54 AM, Roi Dayan <[email protected]> wrote:
>>
>> Fix reset features if probe for meter support fails.
>>
>> Fixes: 92d0d515d67b ("dpif-netlink: Probe for broken Linux meter
>> implementations.")
>> CC: Justin Pettit <[email protected]>
>> Signed-off-by: Roi Dayan <[email protected]>
>> ---
>> lib/dpif-netlink.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>> index a620a6ec52dd..cf34dd9af514 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;
>> }
>
> I agree with Ilya's request to drop the parens around the sizeof. However,
> I'm not sure it's even necessary to do the memset either, since
> dpif_meter_get_features() zeros out the struct before going down the call
> path that eventually calls this function. If you agree, I'd suggest just
> returning immediately. Regardless, though, this is more correct.
>
> Acked-by: Justin Pettit <[email protected]>
>
>
missed that. but maybe I should remove that and keep memset inside?
this is to act the same with the other callback ct_dpif_get_features() ?
the wrappers usually only call the dpif class specific callback.
what do you think?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
