| Message ID | 1525682614-3824-25-git-send-email-ajay.kathat@microchip.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
I feel sort of bad complaining about this patchset when your co-workers already nit picked it to death... :P On Mon, May 07, 2018 at 02:13:28PM +0530, Ajay Singh wrote: > Refactor the code to fix open parenthesis alignment issue reported by > checkpatch.pl script in del_station(). I no idea what an "open parenthesis alignment issue" is. It's sort of surprising because I deal with checkpatch patches a lot. > > Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> > --- > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c > index 4600f4a..7f49d60 100644 > --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c > +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c > @@ -1997,6 +1997,7 @@ static int del_station(struct wiphy *wiphy, struct net_device *dev, > s32 ret = 0; > struct wilc_priv *priv; > struct wilc_vif *vif; > + struct sta_info *info; > > if (!wiphy) > return -EFAULT; > @@ -2004,16 +2005,17 @@ static int del_station(struct wiphy *wiphy, struct net_device *dev, > priv = wiphy_priv(wiphy); > vif = netdev_priv(dev); > > - if (vif->iftype == AP_MODE || vif->iftype == GO_MODE) { > - if (!mac) > - ret = wilc_del_allstation(vif, > - priv->assoc_stainfo.sta_associated_bss); > + if (!(vif->iftype == AP_MODE || vif->iftype == GO_MODE)) I feel like this is better as: if (vif->iftype != AP_MODE && vif->iftype != GO_MODE) > + return ret; What is "ret" here? I haven't looked at this patch in context, but it's probably zero. Just return the literal. regards, dan carpenter
Hi Dan, On Tue, 15 May 2018 12:01:12 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote: > I feel sort of bad complaining about this patchset when your > co-workers already nit picked it to death... :P > > On Mon, May 07, 2018 at 02:13:28PM +0530, Ajay Singh wrote: > > Refactor the code to fix open parenthesis alignment issue reported > > by checkpatch.pl script in del_station(). > > I no idea what an "open parenthesis alignment issue" is. It's sort of > surprising because I deal with checkpatch patches a lot. The exact issue description reported by checkpatch.pl was "CHECK: Alignment should match open parenthesis" To resolve that issue tried by reducing the leading tabs before wilc_del_allstations() call, which helped in resolving checkpatch issue without introducing line over 80 chars. > > > > > Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> > > --- > > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 18 > > ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c > > b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index > > 4600f4a..7f49d60 100644 --- > > a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ > > b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -1997,6 > > +1997,7 @@ static int del_station(struct wiphy *wiphy, struct > > net_device *dev, s32 ret = 0; struct wilc_priv *priv; > > struct wilc_vif *vif; > > + struct sta_info *info; > > > > if (!wiphy) > > return -EFAULT; > > @@ -2004,16 +2005,17 @@ static int del_station(struct wiphy *wiphy, > > struct net_device *dev, priv = wiphy_priv(wiphy); > > vif = netdev_priv(dev); > > > > - if (vif->iftype == AP_MODE || vif->iftype == GO_MODE) { > > - if (!mac) > > - ret = wilc_del_allstation(vif, > > - > > priv->assoc_stainfo.sta_associated_bss); > > + if (!(vif->iftype == AP_MODE || vif->iftype == GO_MODE)) > > I feel like this is better as: > if (vif->iftype != AP_MODE && vif->iftype != GO_MODE) > Thanks for your suggestion. Currently the patch is accepted, i will check and try to include it in future patch as per your inputs. > > + return ret; > > What is "ret" here? I haven't looked at this patch in context, but > it's probably zero. Just return the literal. The value for 'ret' is zero till this point, only if there is failure to post the command in wilc_eneque_cmd() then 'ret' receives -ENOMEM value in below code. Regards, Ajay
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index 4600f4a..7f49d60 100644 --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -1997,6 +1997,7 @@ static int del_station(struct wiphy *wiphy, struct net_device *dev, s32 ret = 0; struct wilc_priv *priv; struct wilc_vif *vif; + struct sta_info *info; if (!wiphy) return -EFAULT; @@ -2004,16 +2005,17 @@ static int del_station(struct wiphy *wiphy, struct net_device *dev, priv = wiphy_priv(wiphy); vif = netdev_priv(dev); - if (vif->iftype == AP_MODE || vif->iftype == GO_MODE) { - if (!mac) - ret = wilc_del_allstation(vif, - priv->assoc_stainfo.sta_associated_bss); + if (!(vif->iftype == AP_MODE || vif->iftype == GO_MODE)) + return ret; - ret = wilc_del_station(vif, mac); + info = &priv->assoc_stainfo; - if (ret) - netdev_err(dev, "Host delete station fail\n"); - } + if (!mac) + ret = wilc_del_allstation(vif, info->sta_associated_bss); + + ret = wilc_del_station(vif, mac); + if (ret) + netdev_err(dev, "Host delete station fail\n"); return ret; }
Refactor the code to fix open parenthesis alignment issue reported by checkpatch.pl script in del_station(). Signed-off-by: Ajay Singh <ajay.kathat@microchip.com> --- drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)