On 2019-05-15 02:32, Jakub Kicinski wrote:
> On Mon, 13 May 2019 15:05:30 +0000, Maxim Mikityanskiy wrote:
>> + err = -EINVAL;
>> +
>> + if (tb[IFLA_INET6_ADDR_GEN_MODE]) {
>> + u8 mode = nla_get_u8(tb[IFLA_INET6_ADDR_GEN_MODE]);
>> +
>> + if (check_addr_gen_mode(mode) < 0)
>> + return -EINVAL;
>> + if (dev && check_stable_privacy(idev, dev_net(dev), mode) < 0)
>> + return -EINVAL;
>> +
>> + err = 0;
>> + }
>> +
>> + if (tb[IFLA_INET6_TOKEN])
>> + err = 0;
>> +
>> + return err;
>
> While at it could you forgo the retval optimization? Most of the time
> it just leads to less readable code for no gain.
OK, I'll make this change in a respin.
> The normal way to write this code would be:
>
> if (!tb[IFLA_INET6_ADDR_GEN_MODE] && !tb[IFLA_INET6_TOKEN])
> return -EINVAL;
Yeah, that's how I wrote this check in RFC 1, but here in this patch I
decided to preserve the pattern that was used in inet6_set_link_af
before my change, to minimize the changes. I agree it's less readable (I
didn't like the error handling flow in inet6_set_link_af either), so
I'll fix it. Thanks for reviewing!
> if (tb[IFLA_INET6_ADDR_GEN_MODE]) {
> u8 mode = nla_get_u8(tb[IFLA_INET6_ADDR_GEN_MODE]);
>
> if (check_addr_gen_mode(mode) < 0)
> return -EINVAL;
> if (dev && check_stable_privacy(idev, dev_net(dev), mode) < 0)
> return -EINVAL;
> }
>
> return 0;
>
