On 27/08/2020 10:23, Michal Kubecek wrote:
> On Mon, Aug 24, 2020 at 08:46:30AM +0200, Hans-Christian Noren Egtvedt wrote:
>> In commit bef780467fa ('ioctl: do not pass transceiver value back to
>> kernel') a regression slipped. If we have a kernel that does not support
>> the ETHTOOL_xLINKSETTINGS API, then the do_ioctl_glinksettings()
>> function will return a NULL pointer.
>>
>> Hence before memset'ing the pointer to zero we must first check it is
>> valid, as NULL return is perfectly fine when running on old kernels.
>>
>
> Please add a standard "Fixes:" line here, i.e.
>
> Fixes: bef780467fa7 ("ioctl: do not pass transceiver value back to kernel")
>
>> Signed-off-by: Hans-Christian Noren Egtvedt <hegtv...@cisco.com>
>> ---
>> ethtool.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/ethtool.c b/ethtool.c
>> index c4ad186..8267d6b 100644
>> --- a/ethtool.c
>> +++ b/ethtool.c
>> @@ -2908,8 +2908,10 @@ static int do_sset(struct cmd_context *ctx)
>> struct ethtool_link_usettings *link_usettings;
>>
>> link_usettings = do_ioctl_glinksettings(ctx);
>> - memset(&link_usettings->deprecated, 0,
>> - sizeof(link_usettings->deprecated));
>> + if (link_usettings) {
>> + memset(&link_usettings->deprecated, 0,
>> + sizeof(link_usettings->deprecated));
>> + }
>> if (link_usettings == NULL)
>> link_usettings = do_ioctl_gset(ctx);
>
> This is correct fix but now we have two if statements with exactly
> opposite conditions here; please combine them into one if-else.
Thank you for the review, a v2 will arrive shortly.
