| Message ID | 20220902084521.3466638-1-casper.casan@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: sparx5: fix return values to correctly use bool | expand |
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
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
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
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,
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(-)