On 03/11/2022 10:06, Roi Dayan wrote:
>
>
> 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?
>
I took "Regardless, though, this is more correct." as this is what you mean.
I sent v2.
thanks.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
