| Message ID | 20210609174353.298731-1-colin.king@canonical.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ab324d8dfddad04bec0e8421242716504e31e204 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [next] net: dsa: sja1105: Fix assigned yet unused return code rc | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Wed, Jun 09, 2021 at 06:43:53PM +0100, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The return code variable rc is being set to return error values in two > places in sja1105_mdiobus_base_tx_register and yet it is not being > returned, the function always returns 0 instead. Fix this by replacing > the return 0 with the return code rc. > > Addresses-Coverity: ("Unused value") > Fixes: 5a8f09748ee7 ("net: dsa: sja1105: register the MDIO buses for 100base-T1 and 100base-TX") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Wed, 9 Jun 2021 18:43:53 +0100 you wrote: > From: Colin Ian King <colin.king@canonical.com> > > The return code variable rc is being set to return error values in two > places in sja1105_mdiobus_base_tx_register and yet it is not being > returned, the function always returns 0 instead. Fix this by replacing > the return 0 with the return code rc. > > [...] Here is the summary with links: - [next] net: dsa: sja1105: Fix assigned yet unused return code rc https://git.kernel.org/netdev/net-next/c/ab324d8dfdda You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
On Wed, Jun 09, 2021 at 06:43:53PM +0100, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The return code variable rc is being set to return error values in two > places in sja1105_mdiobus_base_tx_register and yet it is not being > returned, the function always returns 0 instead. Fix this by replacing > the return 0 with the return code rc. > > Addresses-Coverity: ("Unused value") > Fixes: 5a8f09748ee7 ("net: dsa: sja1105: register the MDIO buses for 100base-T1 and 100base-TX") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/net/dsa/sja1105/sja1105_mdio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c > index 8dfd06318b23..08517c70cb48 100644 > --- a/drivers/net/dsa/sja1105/sja1105_mdio.c > +++ b/drivers/net/dsa/sja1105/sja1105_mdio.c > @@ -171,7 +171,7 @@ static int sja1105_mdiobus_base_tx_register(struct sja1105_private *priv, > out_put_np: > of_node_put(np); > > - return 0; > + return rc; Should this function really return success if of_device_is_available()? regards, dan carpenter
On Thu, Jun 10, 2021 at 09:23:58AM +0300, Dan Carpenter wrote: > On Wed, Jun 09, 2021 at 06:43:53PM +0100, Colin King wrote: > > From: Colin Ian King <colin.king@canonical.com> > > > > The return code variable rc is being set to return error values in two > > places in sja1105_mdiobus_base_tx_register and yet it is not being > > returned, the function always returns 0 instead. Fix this by replacing > > the return 0 with the return code rc. > > > > Addresses-Coverity: ("Unused value") > > Fixes: 5a8f09748ee7 ("net: dsa: sja1105: register the MDIO buses for 100base-T1 and 100base-TX") > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > > --- > > drivers/net/dsa/sja1105/sja1105_mdio.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c > > index 8dfd06318b23..08517c70cb48 100644 > > --- a/drivers/net/dsa/sja1105/sja1105_mdio.c > > +++ b/drivers/net/dsa/sja1105/sja1105_mdio.c > > @@ -171,7 +171,7 @@ static int sja1105_mdiobus_base_tx_register(struct sja1105_private *priv, > > out_put_np: > > of_node_put(np); > > > > - return 0; > > + return rc; > > Should this function really return success if of_device_is_available()? If _not_ of_device_is_available you mean? Yup. Nothing wrong with not having an internal MDIO bus. This is a driver which supports switches that do and switches that don't, and even if the node exists, it may have status = "disabled", which is again fine. Or am I misunderstanding the question?
On Thu, Jun 10, 2021 at 08:51:24PM +0300, Vladimir Oltean wrote: > On Thu, Jun 10, 2021 at 09:23:58AM +0300, Dan Carpenter wrote: > > On Wed, Jun 09, 2021 at 06:43:53PM +0100, Colin King wrote: > > > From: Colin Ian King <colin.king@canonical.com> > > > > > > The return code variable rc is being set to return error values in two > > > places in sja1105_mdiobus_base_tx_register and yet it is not being > > > returned, the function always returns 0 instead. Fix this by replacing > > > the return 0 with the return code rc. > > > > > > Addresses-Coverity: ("Unused value") > > > Fixes: 5a8f09748ee7 ("net: dsa: sja1105: register the MDIO buses for 100base-T1 and 100base-TX") > > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > > > --- > > > drivers/net/dsa/sja1105/sja1105_mdio.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c > > > index 8dfd06318b23..08517c70cb48 100644 > > > --- a/drivers/net/dsa/sja1105/sja1105_mdio.c > > > +++ b/drivers/net/dsa/sja1105/sja1105_mdio.c > > > @@ -171,7 +171,7 @@ static int sja1105_mdiobus_base_tx_register(struct sja1105_private *priv, > > > out_put_np: > > > of_node_put(np); > > > > > > - return 0; > > > + return rc; > > > > Should this function really return success if of_device_is_available()? > > If _not_ of_device_is_available you mean? Yup. Nothing wrong with not > having an internal MDIO bus. This is a driver which supports switches > that do and switches that don't, and even if the node exists, it may > have status = "disabled", which is again fine. > Or am I misunderstanding the question? Yeah. I left out the "!". That answers the question. Thanks! regards, dan carpenter
diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c index 8dfd06318b23..08517c70cb48 100644 --- a/drivers/net/dsa/sja1105/sja1105_mdio.c +++ b/drivers/net/dsa/sja1105/sja1105_mdio.c @@ -171,7 +171,7 @@ static int sja1105_mdiobus_base_tx_register(struct sja1105_private *priv, out_put_np: of_node_put(np); - return 0; + return rc; } static void sja1105_mdiobus_base_tx_unregister(struct sja1105_private *priv)