[PATCH 001] staging: wlan-ng: Add tabstop preceding the statement

Maksymilian Piechota maksymilianpiechota at gmail.com
Tue Jan 31 11:33:33 UTC 2017


On Tue, Jan 31, 2017 at 03:18:45AM -0800, Joe Perches wrote:
> On Tue, 2017-01-31 at 06:04 -0500, Maksymilian Piechota wrote:
> > On Mon, Jan 30, 2017 at 08:00:36PM -0800, Joe Perches wrote:
> > > On Mon, 2017-01-30 at 17:44 +0100, Greg Kroah-Hartman wrote:
> > > > On Mon, Jan 30, 2017 at 11:31:42AM -0500, Maksymilian Piechota wrote:
> > > > > This patch fixes the checkpatch.pl warning:
> > > > > 
> > > > > WARNING: Statements should start on a tabstop
> > > > > 
> > > > > Signed-off-by: Maksymilian Piechota <maksymilianpiechota at gmail.com>
> > > > > ---
> > > > >  drivers/staging/wlan-ng/prism2mgmt.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/staging/wlan-ng/prism2mgmt.c b/drivers/staging/wlan-ng/prism2mgmt.c
> > > > > index 16fb2d3..2d67125 100644
> > > > > --- a/drivers/staging/wlan-ng/prism2mgmt.c
> > > > > +++ b/drivers/staging/wlan-ng/prism2mgmt.c
> > > > > @@ -1308,7 +1308,7 @@ int prism2mgmt_wlansniff(struct wlandevice *wlandev, void *msgp)
> > > > >  			hw->sniffhdr = 0;
> > > > >  			wlandev->netdev->type = ARPHRD_IEEE80211_PRISM;
> > > > >  		} else
> > > > > -		    if ((msg->wlanheader.status ==
> > > > > +			if ((msg->wlanheader.status ==
> > > > >  			 P80211ENUM_msgitem_status_data_ok)
> > > > >  			&& (msg->wlanheader.data == P80211ENUM_truth_true)) {
> > > > >  			hw->sniffhdr = 1;
> > > > 
> > > > Hm, this all doesn't look correct now, does it?  Please fix up the whole
> > > > if statement here.
> > > 
> > > Ideally, it'd look something like:
> > > 	  
> > > 		/* Set the driver state */
> > > 		/* Do we want the prism2 header? */
> > > 		if (msg->prismheader.status == P80211ENUM_msgitem_status_data_ok &&
> > > 		    msg->prismheader.data == P80211ENUM_truth_true) {
> > > 			hw->sniffhdr = 0;
> > > 			wlandev->netdev->type = ARPHRD_IEEE80211_PRISM;
> > > 		} else if (msg->wlanheader.status == P80211ENUM_msgitem_status_data_ok &&
> > > 			   msg->wlanheader.data == P80211ENUM_truth_true) {
> > > 			hw->sniffhdr = 1;
> > > 			wlandev->netdev->type = ARPHRD_IEEE80211_PRISM;
> > > 		} else {
> > > 			wlandev->netdev->type = ARPHRD_IEEE80211;
> > > 		}
> > > 
> > > with the unnecessary parentheses removed,
> > > the logical continuations at the end-of-line,
> > > and the else if on a single line.
> > > 
> > 
> > I must admit it looks better, but this way we get 2 warnings instead of
> > 1 (before my changes). What is the policy? Can we ignore more warnings
> > in order to get cleaner code?
> 
> Yes please.
> 
> checkpatch is just a guide, it's brainless.
> 
> The reason these lines are > 80 columns is
> overly long/verbose identifiers.
> 
> If you really want to clean up the code here,
> the P90211ENUM_ prefixes are a bit misleading
> as they all are #define and not enums at all.
>

But would you like me to remove this prefixes for all of the enums from
p80211types.h? Are you sure it won't cause any symbol conflicts?


More information about the devel mailing list