Messages in this thread | | | Date | Sat, 25 Mar 2023 09:26:31 +0100 | From | Simon Horman <> | Subject | Re: [PATCH net-next v4 01/10] net: sunhme: Fix uninitialized return code |
| |
On Sat, Mar 25, 2023 at 09:17:03AM +0100, Simon Horman wrote: > Hi Sean, > > On Fri, Mar 24, 2023 at 01:51:27PM -0400, Sean Anderson wrote: > > Fix an uninitialized return code if we never found a qfe slot. It would be > > a bug if we ever got into this situation, but it's good to return something > > tracable. > > > > Fixes: acb3f35f920b ("sunhme: forward the error code from pci_enable_device()") > > I think it might be, > > Fixes: 5b3dc6dda6b1 ("sunhme: Regularize probe errors") > > > Reported-by: kernel test robot <lkp@intel.com> > > Reported-by: Dan Carpenter <error27@gmail.com> > > Checkpatch requests a Link tag after Reported-by tags. > > Link: https://lore.kernel.org/oe-kbuild/20230222135715.hjXBN9H5dr7nCnI_Ye2s5H--HsnWom4o9AMScThhDRM@z/T/ > > > Signed-off-by: Sean Anderson <seanga2@gmail.com> > > --- > > > > Changes in v4: > > - Move this fix to its own commit > > > > drivers/net/ethernet/sun/sunhme.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c > > index b0c7ab74a82e..7cf8210ebbec 100644 > > --- a/drivers/net/ethernet/sun/sunhme.c > > +++ b/drivers/net/ethernet/sun/sunhme.c > > @@ -2834,7 +2834,7 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > > int i, qfe_slot = -1; > > char prom_name[64]; > > u8 addr[ETH_ALEN]; > > - int err; > > + int err = -ENODEV; > > > > /* Now make sure pci_dev cookie is there. */ > > #ifdef CONFIG_SPARC > > Unfortunately I don't think this is the right fix, > and indeed smatch still complains with it applied. > > The reason is that a few lines further down there is: > > err = pcim_enable_device(pdev); > if (err) > goto err_out; > > Which overrides the initialisation of err. > Before getting to the line that smatch highlights, correctly as far > as I can tell, as having a missing error code. > > if (qfe_slot == 4) > goto err_out; > > As err_out simply calls 'return err' one could just return here. > And perhaps that is a nice cleanup to roll out at some point.
I see that point is patch 9/10, which also fixes the bug reported by smatch :)
> But to be in keeping with the style of the function, as a minimal fix, > I think the following might be appropriate. > > Note, with this applied err doesn't need to be initialised at the top of > the function (as far as my invocation of smatch is concerned). > > diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c > index b0c7ab74a82e..d6df778a0052 100644 > --- a/drivers/net/ethernet/sun/sunhme.c > +++ b/drivers/net/ethernet/sun/sunhme.c > @@ -2863,8 +2863,10 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > if (!qp->happy_meals[qfe_slot]) > break; > > - if (qfe_slot == 4) > + if (qfe_slot == 4) { > + err = -ENODEV; > goto err_out; > + } > } > > dev = devm_alloc_etherdev(&pdev->dev, sizeof(struct happy_meal)); >
| |