[Outreachy Kernel][PATCH 0/2] staging: wlan-ng: Resolve various warnings reported by checkpatch.pl

[Outreachy Kernel][PATCH 0/2] staging: wlan-ng: Resolve various warnings reported by checkpatch.pl

[Soumyajit Deb]

This patchset resolve warnings issued by checkpatch.pl
This patchset properly aligns the lines of a function call and breaks a
line into two lines to respect the 80 character width constraint. This
helps in maintaining a uniform Linux Kernel coding style.

Soumyajit Deb (2):
  staging: wlang-ng: Properly align the lines of function call
  staging: wlan-ng: Line over 80 characters

 drivers/staging/wlan-ng/cfg80211.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

-- 
2.17.1

[Outreachy Kernel][PATCH 1/2] staging: wlang-ng: Properly align the lines of function call

[Soumyajit Deb]

Align the lines of function call to improve code readability and to
adhere to the uniform Linux Kernel coding style.
Reported by checkpatch.pl

Signed-off-by: Soumyajit Deb <debsoumyajit100@gmail.com>
---
 drivers/staging/wlan-ng/cfg80211.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/wlan-ng/cfg80211.c b/drivers/staging/wlan-ng/cfg80211.c
index fac38c842ac5..3d17037f954e 100644
--- a/drivers/staging/wlan-ng/cfg80211.c
+++ b/drivers/staging/wlan-ng/cfg80211.c
@@ -359,16 +359,15 @@ static int prism2_scan(struct wiphy *wiphy,
 		freq = ieee80211_channel_to_frequency(msg2.dschannel.data,
 						      NL80211_BAND_2GHZ);
 		bss = cfg80211_inform_bss(wiphy,
-			ieee80211_get_channel(wiphy, freq),
-			CFG80211_BSS_FTYPE_UNKNOWN,
-			(const u8 *)&msg2.bssid.data.data,
-			msg2.timestamp.data, msg2.capinfo.data,
-			msg2.beaconperiod.data,
-			ie_buf,
-			ie_len,
-			(msg2.signal.data - 65536) * 100, /* Conversion to signed type */
-			GFP_KERNEL
-		);
+					  ieee80211_get_channel(wiphy, freq),
+					  CFG80211_BSS_FTYPE_UNKNOWN,
+					  (const u8 *)&msg2.bssid.data.data,
+					  msg2.timestamp.data, msg2.capinfo.data,
+					  msg2.beaconperiod.data,
+					  ie_buf,
+					  ie_len,
+					  (msg2.signal.data - 65536) * 100, /* Conversion to signed type */
+					  GFP_KERNEL);
 
 		if (!bss) {
 			err = -ENOMEM;
@@ -473,8 +472,8 @@ static int prism2_connect(struct wiphy *wiphy, struct net_device *dev,
 				return -EINVAL;
 
 			result = prism2_domibset_uint32(wlandev,
-				DIDMIB_DOT11SMT_PRIVACYTABLE_WEPDEFAULTKEYID,
-				sme->key_idx);
+							DIDMIB_DOT11SMT_PRIVACYTABLE_WEPDEFAULTKEYID,
+							sme->key_idx);
 			if (result)
 				goto exit;
 
@@ -586,8 +585,8 @@ static int prism2_set_tx_power(struct wiphy *wiphy, struct wireless_dev *wdev,
 		data = MBM_TO_DBM(mbm);
 
 	result = prism2_domibset_uint32(wlandev,
-		DIDMIB_DOT11PHY_TXPOWERTABLE_CURRENTTXPOWERLEVEL,
-		data);
+					DIDMIB_DOT11PHY_TXPOWERTABLE_CURRENTTXPOWERLEVEL,
+					data);
 
 	if (result) {
 		err = -EFAULT;
-- 
2.17.1

Re: [Outreachy kernel] [Outreachy Kernel][PATCH 1/2] staging: wlang-ng: Properly align the lines of function call

[Stefano Brivio]

On Fri, 27 Mar 2020 20:12:28 +0530
Soumyajit Deb <debsoumyajit100@gmail.com> wrote:

> Align the lines of function call to improve code readability and to
> adhere to the uniform Linux Kernel coding style.
> Reported by checkpatch.pl
> 
> Signed-off-by: Soumyajit Deb <debsoumyajit100@gmail.com>
> ---
>  drivers/staging/wlan-ng/cfg80211.c | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/wlan-ng/cfg80211.c b/drivers/staging/wlan-ng/cfg80211.c
> index fac38c842ac5..3d17037f954e 100644
> --- a/drivers/staging/wlan-ng/cfg80211.c
> +++ b/drivers/staging/wlan-ng/cfg80211.c
> @@ -359,16 +359,15 @@ static int prism2_scan(struct wiphy *wiphy,
>  		freq = ieee80211_channel_to_frequency(msg2.dschannel.data,
>  						      NL80211_BAND_2GHZ);
>  		bss = cfg80211_inform_bss(wiphy,
> -			ieee80211_get_channel(wiphy, freq),
> -			CFG80211_BSS_FTYPE_UNKNOWN,
> -			(const u8 *)&msg2.bssid.data.data,
> -			msg2.timestamp.data, msg2.capinfo.data,
> -			msg2.beaconperiod.data,
> -			ie_buf,
> -			ie_len,
> -			(msg2.signal.data - 65536) * 100, /* Conversion to signed type */
> -			GFP_KERNEL
> -		);
> +					  ieee80211_get_channel(wiphy, freq),
> +					  CFG80211_BSS_FTYPE_UNKNOWN,
> +					  (const u8 *)&msg2.bssid.data.data,
> +					  msg2.timestamp.data, msg2.capinfo.data,
> +					  msg2.beaconperiod.data,
> +					  ie_buf,
> +					  ie_len,
> +					  (msg2.signal.data - 65536) * 100, /* Conversion to signed type */

So... wait a moment:

- March 21, 17:30 UTC: you ask me on IRC how to fix the checkpatch
  warning you're introducing now. You propose to move the comment on a
  line by itself. I suggest that you write a test program and find out
  if this conversion can be done in another way. You say you will do
  that

- March 27, 10:58 UTC: you ask me on IRC if you can use a cast to "i32".
  I have a déjà vu, can't really remember who asked me this, and
  suggest that you write a test program. You say you will do that

- March 27, 12:12 UTC: you send the same question to this list

- March 27, 14:45 UTC: you send this patch

...why, exactly?

-- 
Stefano

Re: [Outreachy kernel] [Outreachy Kernel][PATCH 1/2] staging: wlang-ng: Properly align the lines of function call

[Soumyajit Deb]

[-- Attachment #1: Type: text/plain, Size: 3648 bytes --]

Sorry  for the inconvenience.
Now I am also confused a bit too
But as much as I understand,
this patch is somewhat different from the change I wanted by casting to i32.

This patch just addresses the warning of properly aligning the lines as
reported by checkpatch.pl.
While, the change of casting to i32 was for line over 80 characters
warning, which is a different warning altogether, so that change must be
addressed in some other patch.
I wanted to cast to i32 so that I can shorten the line and resolve the
warning of line over 80 character as reported by checkpatch.pl

Sorry, for any misunderstanding. I am very new to kernel development and
was confused a bit about the process then.
Sorry for any mistake on my part.


On Mon, 30 Mar 2020 at 14:43, Stefano Brivio <sbrivio@redhat.com> wrote:

> On Fri, 27 Mar 2020 20:12:28 +0530
> Soumyajit Deb <debsoumyajit100@gmail.com> wrote:
>
> > Align the lines of function call to improve code readability and to
> > adhere to the uniform Linux Kernel coding style.
> > Reported by checkpatch.pl
> >
> > Signed-off-by: Soumyajit Deb <debsoumyajit100@gmail.com>
> > ---
> >  drivers/staging/wlan-ng/cfg80211.c | 27 +++++++++++++--------------
> >  1 file changed, 13 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/staging/wlan-ng/cfg80211.c
> b/drivers/staging/wlan-ng/cfg80211.c
> > index fac38c842ac5..3d17037f954e 100644
> > --- a/drivers/staging/wlan-ng/cfg80211.c
> > +++ b/drivers/staging/wlan-ng/cfg80211.c
> > @@ -359,16 +359,15 @@ static int prism2_scan(struct wiphy *wiphy,
> >               freq = ieee80211_channel_to_frequency(msg2.dschannel.data,
> >                                                     NL80211_BAND_2GHZ);
> >               bss = cfg80211_inform_bss(wiphy,
> > -                     ieee80211_get_channel(wiphy, freq),
> > -                     CFG80211_BSS_FTYPE_UNKNOWN,
> > -                     (const u8 *)&msg2.bssid.data.data,
> > -                     msg2.timestamp.data, msg2.capinfo.data,
> > -                     msg2.beaconperiod.data,
> > -                     ie_buf,
> > -                     ie_len,
> > -                     (msg2.signal.data - 65536) * 100, /* Conversion to
> signed type */
> > -                     GFP_KERNEL
> > -             );
> > +                                       ieee80211_get_channel(wiphy,
> freq),
> > +                                       CFG80211_BSS_FTYPE_UNKNOWN,
> > +                                       (const u8
> *)&msg2.bssid.data.data,
> > +                                       msg2.timestamp.data,
> msg2.capinfo.data,
> > +                                       msg2.beaconperiod.data,
> > +                                       ie_buf,
> > +                                       ie_len,
> > +                                       (msg2.signal.data - 65536) *
> 100, /* Conversion to signed type */
>
> So... wait a moment:
>
> - March 21, 17:30 UTC: you ask me on IRC how to fix the checkpatch
>   warning you're introducing now. You propose to move the comment on a
>   line by itself. I suggest that you write a test program and find out
>   if this conversion can be done in another way. You say you will do
>   that
>
> - March 27, 10:58 UTC: you ask me on IRC if you can use a cast to "i32".
>   I have a déjà vu, can't really remember who asked me this, and
>   suggest that you write a test program. You say you will do that
>
> - March 27, 12:12 UTC: you send the same question to this list
>
> - March 27, 14:45 UTC: you send this patch
>
> ...why, exactly?
>
> --
> Stefano
>
>

[-- Attachment #2: Type: text/html, Size: 4907 bytes --]

Re: [Outreachy kernel] [Outreachy Kernel][PATCH 1/2] staging: wlang-ng: Properly align the lines of function call

[Soumyajit Deb]

[-- Attachment #1: Type: text/plain, Size: 3955 bytes --]

Is there any issue with this patch?
Please let me know, so I can rectify them and send v2 of the patch if
needed.

On Mon, 30 Mar 2020 at 15:25, Soumyajit Deb <debsoumyajit100@gmail.com>
wrote:

> Sorry  for the inconvenience.
> Now I am also confused a bit too
> But as much as I understand,
> this patch is somewhat different from the change I wanted by casting to
> i32.
>
> This patch just addresses the warning of properly aligning the lines as
> reported by checkpatch.pl.
> While, the change of casting to i32 was for line over 80 characters
> warning, which is a different warning altogether, so that change must be
> addressed in some other patch.
> I wanted to cast to i32 so that I can shorten the line and resolve the
> warning of line over 80 character as reported by checkpatch.pl
>
> Sorry, for any misunderstanding. I am very new to kernel development and
> was confused a bit about the process then.
> Sorry for any mistake on my part.
>
>
> On Mon, 30 Mar 2020 at 14:43, Stefano Brivio <sbrivio@redhat.com> wrote:
>
>> On Fri, 27 Mar 2020 20:12:28 +0530
>> Soumyajit Deb <debsoumyajit100@gmail.com> wrote:
>>
>> > Align the lines of function call to improve code readability and to
>> > adhere to the uniform Linux Kernel coding style.
>> > Reported by checkpatch.pl
>> >
>> > Signed-off-by: Soumyajit Deb <debsoumyajit100@gmail.com>
>> > ---
>> >  drivers/staging/wlan-ng/cfg80211.c | 27 +++++++++++++--------------
>> >  1 file changed, 13 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/drivers/staging/wlan-ng/cfg80211.c
>> b/drivers/staging/wlan-ng/cfg80211.c
>> > index fac38c842ac5..3d17037f954e 100644
>> > --- a/drivers/staging/wlan-ng/cfg80211.c
>> > +++ b/drivers/staging/wlan-ng/cfg80211.c
>> > @@ -359,16 +359,15 @@ static int prism2_scan(struct wiphy *wiphy,
>> >               freq = ieee80211_channel_to_frequency(msg2.dschannel.data,
>> >                                                     NL80211_BAND_2GHZ);
>> >               bss = cfg80211_inform_bss(wiphy,
>> > -                     ieee80211_get_channel(wiphy, freq),
>> > -                     CFG80211_BSS_FTYPE_UNKNOWN,
>> > -                     (const u8 *)&msg2.bssid.data.data,
>> > -                     msg2.timestamp.data, msg2.capinfo.data,
>> > -                     msg2.beaconperiod.data,
>> > -                     ie_buf,
>> > -                     ie_len,
>> > -                     (msg2.signal.data - 65536) * 100, /* Conversion
>> to signed type */
>> > -                     GFP_KERNEL
>> > -             );
>> > +                                       ieee80211_get_channel(wiphy,
>> freq),
>> > +                                       CFG80211_BSS_FTYPE_UNKNOWN,
>> > +                                       (const u8
>> *)&msg2.bssid.data.data,
>> > +                                       msg2.timestamp.data,
>> msg2.capinfo.data,
>> > +                                       msg2.beaconperiod.data,
>> > +                                       ie_buf,
>> > +                                       ie_len,
>> > +                                       (msg2.signal.data - 65536) *
>> 100, /* Conversion to signed type */
>>
>> So... wait a moment:
>>
>> - March 21, 17:30 UTC: you ask me on IRC how to fix the checkpatch
>>   warning you're introducing now. You propose to move the comment on a
>>   line by itself. I suggest that you write a test program and find out
>>   if this conversion can be done in another way. You say you will do
>>   that
>>
>> - March 27, 10:58 UTC: you ask me on IRC if you can use a cast to "i32".
>>   I have a déjà vu, can't really remember who asked me this, and
>>   suggest that you write a test program. You say you will do that
>>
>> - March 27, 12:12 UTC: you send the same question to this list
>>
>> - March 27, 14:45 UTC: you send this patch
>>
>> ...why, exactly?
>>
>> --
>> Stefano
>>
>>

[-- Attachment #2: Type: text/html, Size: 5465 bytes --]

Re: [Outreachy kernel] [Outreachy Kernel][PATCH 1/2] staging: wlang-ng: Properly align the lines of function call

[Stefano Brivio]

On Mon, 30 Mar 2020 15:44:43 +0530
Soumyajit Deb <debsoumyajit100@gmail.com> wrote:

> Is there any issue with this patch?

Yes, it makes the code even less readable because it reaches 115
columns.

-- 
Stefano

Re: [Outreachy kernel] [Outreachy Kernel][PATCH 1/2] staging: wlang-ng: Properly align the lines of function call

[Stefano Brivio]

Soumyajit,

On Mon, 30 Mar 2020 15:25:03 +0530
Soumyajit Deb <debsoumyajit100@gmail.com> wrote:

> Sorry  for the inconvenience.
> Now I am also confused a bit too
> But as much as I understand,
> this patch is somewhat different from the change I wanted by casting to i32.

Can you please stop top-quoting first? This:
	https://en.wikipedia.org/wiki/Posting_style#Bottom-posting

is the preferred way to comment on code and intervene in discussions on
the communities we'll be working with.

> This patch just addresses the warning of properly aligning the lines as
> reported by checkpatch.pl.

...but it makes the problem worse, because instead of just 89 columns
you reach 115 columns and you also exceed 80 columns with code, not
just with comments. That's how the two things are (obviously) related.

> While, the change of casting to i32 was for line over 80 characters
> warning, which is a different warning altogether, so that change must be
> addressed in some other patch.
> I wanted to cast to i32 so that I can shorten the line and resolve the
> warning of line over 80 character as reported by checkpatch.pl
> 
> Sorry, for any misunderstanding. I am very new to kernel development and
> was confused a bit about the process then.
> Sorry for any mistake on my part.

No, sorry, but if you ask about something three times, and three times I
give you the same answer, well, the confusion will start happening on my
side.

I understand you might be tempted to find the easiest possible patches
to have a number of formal submissions, instead of losing your time
writing a simple test program (which would tell you that 'i32' doesn't
exist, by the way). I don't think it's a reasonable approach, though.

-- 
Stefano

[Outreachy Kernel][PATCH 2/2] staging: wlan-ng: Line over 80 characters

[Soumyajit Deb]

Move the last argument of the line to the next line to prevent the line
from exceeding 80 character width limit.
Reported by checkpatch.pl

Signed-off-by: Soumyajit Deb <debsoumyajit100@gmail.com>
---
 drivers/staging/wlan-ng/cfg80211.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/wlan-ng/cfg80211.c b/drivers/staging/wlan-ng/cfg80211.c
index 3d17037f954e..9ab57822979b 100644
--- a/drivers/staging/wlan-ng/cfg80211.c
+++ b/drivers/staging/wlan-ng/cfg80211.c
@@ -362,7 +362,8 @@ static int prism2_scan(struct wiphy *wiphy,
 					  ieee80211_get_channel(wiphy, freq),
 					  CFG80211_BSS_FTYPE_UNKNOWN,
 					  (const u8 *)&msg2.bssid.data.data,
-					  msg2.timestamp.data, msg2.capinfo.data,
+					  msg2.timestamp.data,
+					  msg2.capinfo.data,
 					  msg2.beaconperiod.data,
 					  ie_buf,
 					  ie_len,
-- 
2.17.1