Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1580839 > unrolled thread

[PATCH v2 6/9] staging: rtl8192u: Fixing coding style issues

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 6/9] staging: rtl8192u: Fixing coding style issues simran singhal <singhalsimran0@gmail.com> - 2017-02-14 21:20 +0100
    Re: [PATCH v2 6/9] staging: rtl8192u: Fixing coding style issues Joe Perches <joe@perches.com> - 2017-02-14 21:30 +0100
      Re: [PATCH v2 6/9] staging: rtl8192u: Fixing coding style issues SIMRAN SINGHAL <singhalsimran0@gmail.com> - 2017-02-15 04:30 +0100
        Re: [PATCH v2 6/9] staging: rtl8192u: Fixing coding style issues Joe Perches <joe@perches.com> - 2017-02-15 04:40 +0100
          Re: [PATCH v2 6/9] staging: rtl8192u: Fixing coding style issues SIMRAN SINGHAL <singhalsimran0@gmail.com> - 2017-02-15 09:20 +0100
            Re: [PATCH v2 6/9] staging: rtl8192u: Fixing coding style issues Joe Perches <joe@perches.com> - 2017-02-15 10:30 +0100
            Re: [PATCH v2 6/9] staging: rtl8192u: Fixing coding style issues Greg KH <gregkh@linuxfoundation.org> - 2017-02-16 19:40 +0100
              Re: [PATCH v2 6/9] staging: rtl8192u: Fixing coding style issues SIMRAN SINGHAL <singhalsimran0@gmail.com> - 2017-02-16 20:10 +0100

#1580839 — [PATCH v2 6/9] staging: rtl8192u: Fixing coding style issues

fixed errors and warnings:
ERROR: spaces required around that '='
ERROR: spaces required around that '<'
ERROR: space required before the open parenthesis '('
CHECK: spaces preferred around that '&'
CHECK: spaces preferred around that '<<'
ERROR: space required after that ','
ERROR: spaces required around that '+='
WARNING: Missing a blank line after declarations
CHECK: spaces required around that '?'
CHECK: spaces required around that ':'

Signed-off-by: simran singhal <singhalsimran0@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
index fff2020..c846223 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
@@ -85,10 +85,10 @@ static inline char *rtl819x_translate_scan(struct ieee80211_device *ieee,
 	}
 	/* Add the protocol name */
 	iwe.cmd = SIOCGIWNAME;
-	for(i=0; i<ARRAY_SIZE(ieee80211_modes); i++) {
-		if(network->mode&(1<<i)) {
-			sprintf(pname,ieee80211_modes[i].mode_string,ieee80211_modes[i].mode_size);
-			pname +=ieee80211_modes[i].mode_size;
+	for (i = 0; i < ARRAY_SIZE(ieee80211_modes); i++) {
+		if (network->mode & (1 << i)) {
+			sprintf(pname, ieee80211_modes[i].mode_string, ieee80211_modes[i].mode_size);
+			pname += ieee80211_modes[i].mode_size;
 		}
 	}
 	*pname = '\0';
@@ -150,14 +150,15 @@ static inline char *rtl819x_translate_scan(struct ieee80211_device *ieee,
 		PHT_CAPABILITY_ELE ht_cap = NULL;
 		bool is40M = false, isShortGI = false;
 		u8 max_mcs = 0;
+
 		if (!memcmp(network->bssht.bdHTCapBuf, EWC11NHTCap, 4))
 			ht_cap = (PHT_CAPABILITY_ELE)&network->bssht.bdHTCapBuf[4];
 		else
 			ht_cap = (PHT_CAPABILITY_ELE)&network->bssht.bdHTCapBuf[0];
-		is40M = (ht_cap->ChlWidth)?1:0;
-		isShortGI = (ht_cap->ChlWidth)?
-						((ht_cap->ShortGI40Mhz)?1:0):
-						((ht_cap->ShortGI20Mhz)?1:0);
+		is40M = (ht_cap->ChlWidth) ? 1 : 0;
+		isShortGI = (ht_cap->ChlWidth) ?
+						((ht_cap->ShortGI40Mhz) ? 1 : 0) :
+						((ht_cap->ShortGI20Mhz) ? 1 : 0);
 
 		max_mcs = HTGetHighestMCSRate(ieee, ht_cap->MCS, MCS_FILTER_ALL);
 		rate = MCS_DATA_RATE[is40M][isShortGI][max_mcs&0x7f];
-- 
2.7.4

[toc] | [next] | [standalone]

#1580844

On Wed, 2017-02-15 at 01:44 +0530, simran singhal wrote:
[]
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
[]
> @@ -150,14 +150,15 @@ static inline char *rtl819x_translate_scan(struct ieee80211_device *ieee,
>  		PHT_CAPABILITY_ELE ht_cap = NULL;
>  		bool is40M = false, isShortGI = false;
>  		u8 max_mcs = 0;
> +
>  		if (!memcmp(network->bssht.bdHTCapBuf, EWC11NHTCap, 4))
>  			ht_cap = (PHT_CAPABILITY_ELE)&network->bssht.bdHTCapBuf[4];
>  		else
>  			ht_cap = (PHT_CAPABILITY_ELE)&network->bssht.bdHTCapBuf[0];
> -		is40M = (ht_cap->ChlWidth)?1:0;
> -		isShortGI = (ht_cap->ChlWidth)?
> -						((ht_cap->ShortGI40Mhz)?1:0):
> -						((ht_cap->ShortGI20Mhz)?1:0);
> +		is40M = (ht_cap->ChlWidth) ? 1 : 0;
> +		isShortGI = (ht_cap->ChlWidth) ?
> +						((ht_cap->ShortGI40Mhz) ? 1 : 0) :
> +						((ht_cap->ShortGI20Mhz) ? 1 : 0);

You have a brain, checkpatch is brainless.
Please remember to do more than just shut-up checkpatch.

This could be simplified by removing parentheses and ternaries to

		isShortGI = ht_cap->Ch1Width ? ht_cap->ShortGI40Mhz
					     : ht_cap->ShortGI20Mhz;

[toc] | [prev] | [next] | [standalone]

#1581034

Yes, I totally agree with joe.

But as there is no coding style issue in this patch.
So, do I have to resend the complete patch series again?


On Wed, Feb 15, 2017 at 1:53 AM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2017-02-15 at 01:44 +0530, simran singhal wrote:
> []
>> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_wx.c
> []
>> @@ -150,14 +150,15 @@ static inline char *rtl819x_translate_scan(struct ieee80211_device *ieee,
>>               PHT_CAPABILITY_ELE ht_cap = NULL;
>>               bool is40M = false, isShortGI = false;
>>               u8 max_mcs = 0;
>> +
>>               if (!memcmp(network->bssht.bdHTCapBuf, EWC11NHTCap, 4))
>>                       ht_cap = (PHT_CAPABILITY_ELE)&network->bssht.bdHTCapBuf[4];
>>               else
>>                       ht_cap = (PHT_CAPABILITY_ELE)&network->bssht.bdHTCapBuf[0];
>> -             is40M = (ht_cap->ChlWidth)?1:0;
>> -             isShortGI = (ht_cap->ChlWidth)?
>> -                                             ((ht_cap->ShortGI40Mhz)?1:0):
>> -                                             ((ht_cap->ShortGI20Mhz)?1:0);
>> +             is40M = (ht_cap->ChlWidth) ? 1 : 0;
>> +             isShortGI = (ht_cap->ChlWidth) ?
>> +                                             ((ht_cap->ShortGI40Mhz) ? 1 : 0) :
>> +                                             ((ht_cap->ShortGI20Mhz) ? 1 : 0);
>
> You have a brain, checkpatch is brainless.
> Please remember to do more than just shut-up checkpatch.
>
> This could be simplified by removing parentheses and ternaries to
>
>                 isShortGI = ht_cap->Ch1Width ? ht_cap->ShortGI40Mhz
>                                              : ht_cap->ShortGI20Mhz;
>
>

[toc] | [prev] | [next] | [standalone]

#1581039

On Wed, 2017-02-15 at 08:52 +0530, SIMRAN SINGHAL wrote:
> Yes, I totally agree with joe.
> 
> But as there is no coding style issue in this patch.

Style is more than just checkpatch conformity.

> So, do I have to resend the complete patch series again?

I'm not applying any of these so that's entirely up
to Greg.

Perhaps he might apply the first 5 patches and reject
this one, maybe he might apply all the patches.

In any case, do strive to develop some reasonable
semblance of style and do more than just checkpatch
suggested patches and find and fix some actual defect
somewhere.

[toc] | [prev] | [next] | [standalone]

#1581112

On Wed, Feb 15, 2017 at 9:01 AM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2017-02-15 at 08:52 +0530, SIMRAN SINGHAL wrote:
>> Yes, I totally agree with joe.
>>
>> But as there is no coding style issue in this patch.
>
> Style is more than just checkpatch conformity.
>
>> So, do I have to resend the complete patch series again?
>
> I'm not applying any of these so that's entirely up
> to Greg.
>
> Perhaps he might apply the first 5 patches and reject
> this one, maybe he might apply all the patches.
>
> In any case, do strive to develop some reasonable
> semblance of style and do more than just checkpatch
> suggested patches and find and fix some actual defect
> somewhere.

Joe, I am new to Linux kernel and just have basic knowledge of
 C and C++.
For doing more than checkpatches I need to explore Linux kernel more,
so please suggest me some tasks that help me in exploring
Linux kernel, which helps me in building the better understanding of codes,
which I am finding difficult to understand so that I can do more than
just checkpatch.

Thanks,
Simran

[toc] | [prev] | [next] | [standalone]

#1581159

On Wed, 2017-02-15 at 13:48 +0530, SIMRAN SINGHAL wrote:
> Joe, I am new to Linux kernel and just have basic knowledge of
>  C and C++.
> For doing more than checkpatches I need to explore Linux kernel more,
> so please suggest me some tasks that help me in exploring
> Linux kernel, which helps me in building the better understanding of codes,
> which I am finding difficult to understand so that I can do more than
> just checkpatch.

Linux kernel isn't a great project to learn c/c++ as
it is a relatively old, complex project without a lo
of obvious defects.

see: http://kernelnewbies.org

Make sure you read the kernel newbies lists and the
kernel process help files in Documentation/process/.

In the drivers/staging directory tree are many TODO
files.  Read some of them.

List of kernel related bugs to fix:
https://bugzilla.kernel.org/buglist.cgi?bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=VERIFIED&bug_status=DEFERRED&bug_status=NEEDINFO&bugidtype=include&chfieldto=Now&cmdtype=doit&emailassigned_to1=1&emailassigned_to2=1&emailcc2=1&emailreporter2=1&emailtype1=substring&emailtype2=substring&field0-0-0=noop&kernel_version_type=allwordssubstr&long_desc_type=substring&order=Reuse%20same%20sort%20as%20last%20time&query_format=advanced&regression=both&short_desc_type=allwordssubstr&type0-0-0=noop

[toc] | [prev] | [next] | [standalone]

#1582844

On Wed, Feb 15, 2017 at 01:48:13PM +0530, SIMRAN SINGHAL wrote:
> On Wed, Feb 15, 2017 at 9:01 AM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2017-02-15 at 08:52 +0530, SIMRAN SINGHAL wrote:
> >> Yes, I totally agree with joe.
> >>
> >> But as there is no coding style issue in this patch.
> >
> > Style is more than just checkpatch conformity.
> >
> >> So, do I have to resend the complete patch series again?
> >
> > I'm not applying any of these so that's entirely up
> > to Greg.
> >
> > Perhaps he might apply the first 5 patches and reject
> > this one, maybe he might apply all the patches.
> >
> > In any case, do strive to develop some reasonable
> > semblance of style and do more than just checkpatch
> > suggested patches and find and fix some actual defect
> > somewhere.
> 
> Joe, I am new to Linux kernel and just have basic knowledge of
>  C and C++.

As Joe said, the kernel is _not_ the project to learn C with.  Please
get some more experience with userspace programs in C first, before
working on the kernel.  It will be much easier for you.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1582868

I didn't mean what you get. I just want to say I am a beginner and like to
contribute to Linux kernel. But at present, I am in learning mode.

I never mean that I am doing this for learning C or C++ but yes for
contributing to
Linux kernel in every possible way.

And, would be very happy if you guide me so that I can improve.




On Fri, Feb 17, 2017 at 12:02 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Wed, Feb 15, 2017 at 01:48:13PM +0530, SIMRAN SINGHAL wrote:
>> On Wed, Feb 15, 2017 at 9:01 AM, Joe Perches <joe@perches.com> wrote:
>> > On Wed, 2017-02-15 at 08:52 +0530, SIMRAN SINGHAL wrote:
>> >> Yes, I totally agree with joe.
>> >>
>> >> But as there is no coding style issue in this patch.
>> >
>> > Style is more than just checkpatch conformity.
>> >
>> >> So, do I have to resend the complete patch series again?
>> >
>> > I'm not applying any of these so that's entirely up
>> > to Greg.
>> >
>> > Perhaps he might apply the first 5 patches and reject
>> > this one, maybe he might apply all the patches.
>> >
>> > In any case, do strive to develop some reasonable
>> > semblance of style and do more than just checkpatch
>> > suggested patches and find and fix some actual defect
>> > somewhere.
>>
>> Joe, I am new to Linux kernel and just have basic knowledge of
>>  C and C++.
>
> As Joe said, the kernel is _not_ the project to learn C with.  Please
> get some more experience with userspace programs in C first, before
> working on the kernel.  It will be much easier for you.
>
> thanks,
>
> greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web