[PATCH net-next] net: sparx5: fix return values to correctly use bool

[PATCH net-next] net: sparx5: fix return values to correctly use bool

[Casper Andersson]

Function was declared to return bool, but used error return strategy (0
for success, else error). Now correctly uses bool to indicate whether
the entry was found or not.

Does not have any impact on functionality.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Casper Andersson <casper.casan@gmail.com>
---
 .../ethernet/microchip/sparx5/sparx5_mactable.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
index a5837dbe0c7e..4bc80bc7475f 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
@@ -189,7 +189,8 @@ bool sparx5_mact_getnext(struct sparx5 *sparx5,
 bool sparx5_mact_find(struct sparx5 *sparx5,
 		      const unsigned char mac[ETH_ALEN], u16 vid, u32 *pcfg2)
 {
-	int ret;
+	bool found = false;
+	int err;
 	u32 cfg2;
 
 	mutex_lock(&sparx5->lock);
@@ -201,18 +202,18 @@ bool sparx5_mact_find(struct sparx5 *sparx5,
 		LRN_COMMON_ACCESS_CTRL_MAC_TABLE_ACCESS_SHOT_SET(1),
 		sparx5, LRN_COMMON_ACCESS_CTRL);
 
-	ret = sparx5_mact_wait_for_completion(sparx5);
-	if (ret == 0) {
+	err = sparx5_mact_wait_for_completion(sparx5);
+	if (!err) {
 		cfg2 = spx5_rd(sparx5, LRN_MAC_ACCESS_CFG_2);
-		if (LRN_MAC_ACCESS_CFG_2_MAC_ENTRY_VLD_GET(cfg2))
+		if (LRN_MAC_ACCESS_CFG_2_MAC_ENTRY_VLD_GET(cfg2)) {
 			*pcfg2 = cfg2;
-		else
-			ret = -ENOENT;
+			found = true;
+		}
 	}
 
 	mutex_unlock(&sparx5->lock);
 
-	return ret;
+	return found;
 }
 
 int sparx5_mact_forget(struct sparx5 *sparx5,
@@ -296,7 +297,7 @@ int sparx5_add_mact_entry(struct sparx5 *sparx5,
 	u32 cfg2;
 
 	ret = sparx5_mact_find(sparx5, addr, vid, &cfg2);
-	if (!ret)
+	if (ret)
 		return 0;
 
 	/* In case the entry already exists, don't add it again to SW,
-- 
2.34.1

Re: [PATCH net-next] net: sparx5: fix return values to correctly use bool

[Andrew Lunn]

On Fri, Sep 02, 2022 at 10:45:21AM +0200, Casper Andersson wrote:
> Function was declared to return bool, but used error return strategy (0
> for success, else error). Now correctly uses bool to indicate whether
> the entry was found or not.

I think it would be better to actually return an int. < 0 error, 0 =
not foumd > 1 found. You can then return ETIMEDOUT etc.

    Andrew

Re: [PATCH net-next] net: sparx5: fix return values to correctly use bool

[Casper Andersson]

Hi,

On 2022-09-02 18:41, Andrew Lunn wrote:
> On Fri, Sep 02, 2022 at 10:45:21AM +0200, Casper Andersson wrote:
> > Function was declared to return bool, but used error return strategy (0
> > for success, else error). Now correctly uses bool to indicate whether
> > the entry was found or not.
> 
> I think it would be better to actually return an int. < 0 error, 0 =
> not foumd > 1 found. You can then return ETIMEDOUT etc.
> 
>     Andrew

I can submit a new version with this. But since the commit title will be
different I assume I should make it a new patch and not a v2.

Best Regards,
Casper

Re: [PATCH net-next] net: sparx5: fix return values to correctly use bool

[Andrew Lunn]

On Mon, Sep 05, 2022 at 05:33:44PM +0200, Casper Andersson wrote:
> Hi,
> 
> On 2022-09-02 18:41, Andrew Lunn wrote:
> > On Fri, Sep 02, 2022 at 10:45:21AM +0200, Casper Andersson wrote:
> > > Function was declared to return bool, but used error return strategy (0
> > > for success, else error). Now correctly uses bool to indicate whether
> > > the entry was found or not.
> > 
> > I think it would be better to actually return an int. < 0 error, 0 =
> > not foumd > 1 found. You can then return ETIMEDOUT etc.
> > 
> >     Andrew
> 
> I can submit a new version with this. But since the commit title will be
> different I assume I should make it a new patch and not a v2.

I don't think it matters if it is a new patch, or a v2. There is some
attempts to track patches through their revisions, but subjects do
change as patches get revised, so it is never a precise things.

       Andrew