Hi, On 11/30/23 17:24, Vadim Pasternak wrote: > Hi Dan, > >> -----Original Message----- >> From: Dan Carpenter <dan.carpenter@xxxxxxxxxx> >> Sent: Thursday, 30 November 2023 13:47 >> To: Yu Sun <u202112062@xxxxxxxxxxx> >> Cc: Hans de Goede <hdegoede@xxxxxxxxxx>; Ilpo Järvinen >> <ilpo.jarvinen@xxxxxxxxxxxxxxx>; Mark Gross <markgross@xxxxxxxxxx>; Vadim >> Pasternak <vadimp@xxxxxxxxxx>; hust-os-kernel- >> patches@xxxxxxxxxxxxxxxx; Dongliang Mu <dzm91@xxxxxxxxxxx>; Dan >> Carpenter <error27@xxxxxxxxx>; platform-driver-x86@xxxxxxxxxxxxxxx; linux- >> kernel@xxxxxxxxxxxxxxx >> Subject: Re: [PATCH] platform/mellanox: mlxreg-lc: Check before variable >> dereferenced >> >> On Thu, Nov 30, 2023 at 05:44:07PM +0800, Yu Sun wrote: >>> there is a warning saying variable dereferenced before check >>> 'data->notifier' in line 828. >>> add "for(data->notifier)" before variable deferenced. >> ^^^ >> Should have been "if (data->notifier)". >> >>> >>> Signed-off-by: Yu Sun <u202112062@xxxxxxxxxxx> >>> Reviewed-by: Dongliang Mu <dzm91@xxxxxxxxxxx> >>> Reviewed-by: Dan Carpenter <error27@xxxxxxxxx> >> >> I didn't really explicitly give a Reviewed-by tag for this patch. >> https://groups.google.com/g/hust-os-kernel- >> patches/c/c5hUaYIDcII/m/h4aFS7PkCQAJ >> I also said that I thought it looked correct but that it needed a Fixes: >> tag however the Fixes tag I suggested was wrong. >> >> Looking at it now, the correct Fixes tag would be: >> Fixes: 1c8ee06b637f ("platform/mellanox: Remove unnecessary code") >> >> That commit says that the NULL check is not required. So now I'm confused. >> On the one hand, the impulse is to trust the maintainer, but on the other hand >> my review suggested that the NULL check might be required. > > Yes, it indeed required. > My mistake. Ok, so we are going to need a v2 of this addressing Dan's remarks about the commit message. Yu Sun, can you please submit a new version addressing Dan's comments on the commit message ? Regards, Hans
