Submitted a first patch and no reply

Submitted a first patch and no reply

[shirish gajera]

Hi,

I have submitted my first patch on 12/30/2014 and I have not received any
reply for next step like error or merge ect.

I send the email to person that get_maintainer.pl script tells me to send.

Please guide whom should I reached for this.

 Below is the email
---------
This patch fixes the checkpatch.pl warning:

WARNING: Single statement macros should not use a do {} while (0) loop

I have added single statement in curly braces, because it was giving
me "WARNING: macros should not use a trailing semicolon".

Signed-off-by: Shirish Gajera <gajerashirish@gmail.com>
---
 drivers/staging/skein/skein_block.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/skein/skein_block.c
b/drivers/staging/skein/skein_block.c
index 66261ab..5f46cd6 100644
--- a/drivers/staging/skein/skein_block.c
+++ b/drivers/staging/skein/skein_block.c
@@ -82,10 +82,10 @@ do {
                  \
 } while (0)
 #else
 /* looping version */
-#define R256(p0, p1, p2, p3, ROT, r_num) \
-do { \
-       ROUND256(p0, p1, p2, p3, ROT, r_num); \
-} while (0)
+#define R256(p0, p1, p2, p3, ROT, r_num)        \
+{                                              \
+       ROUND256(p0, p1, p2, p3, ROT, r_num);   \
+}

 #define I256(R) \
 do { \

---------------------------


Thanks,
Shirish
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150108/63ccf957/attachment.html 

Submitted a first patch and no reply

[Valdis.Kletnieks at vt.edu]

On Thu, 08 Jan 2015 12:00:07 -0800, shirish gajera said:

> WARNING: Single statement macros should not use a do {} while (0) loop
>
> I have added single statement in curly braces, because it was giving
> me "WARNING: macros should not use a trailing semicolon".
>
> Signed-off-by: Shirish Gajera <gajerashirish@gmail.com>
> ---
>  drivers/staging/skein/skein_block.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/skein/skein_block.c
> b/drivers/staging/skein/skein_block.c
> index 66261ab..5f46cd6 100644
> --- a/drivers/staging/skein/skein_block.c
> +++ b/drivers/staging/skein/skein_block.c
> @@ -82,10 +82,10 @@ do {
>                   \
>  } while (0)
>  #else
>  /* looping version */
> -#define R256(p0, p1, p2, p3, ROT, r_num) \

Why did you not fix the definition of R256 in the other half of
the "#if SKEIN_UNROLL_256 == 0" code?  Or the definitions for R512
and R1024?  You cleaned up one case, and left another 5 cases alone....

(This is part of why blindly fixing checkpatch complaints without actually
looking at the code and understanding isn't always a good thing)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150108/c058b11b/attachment.bin 

Submitted a first patch and no reply

[shirish gajera]

Hi,

Actually on the website it's return that

Pick a warning, and try to fix it. For your first patch, only pick one
warning. In the future you can group multiple changes into one patch, but
only if you follow the PatchPhilosophy
<http://kernelnewbies.org/PatchPhilosophy> of breaking each patch into
logical changes.


That's hwy I just fix one warning.


Please let me know if I can fix more than one warning in my first patch.


Thanks,

Shirish

On Thu, Jan 8, 2015 at 12:32 PM, <Valdis.Kletnieks@vt.edu> wrote:

> On Thu, 08 Jan 2015 12:00:07 -0800, shirish gajera said:
>
> > WARNING: Single statement macros should not use a do {} while (0) loop
> >
> > I have added single statement in curly braces, because it was giving
> > me "WARNING: macros should not use a trailing semicolon".
> >
> > Signed-off-by: Shirish Gajera <gajerashirish@gmail.com>
> > ---
> >  drivers/staging/skein/skein_block.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/skein/skein_block.c
> > b/drivers/staging/skein/skein_block.c
> > index 66261ab..5f46cd6 100644
> > --- a/drivers/staging/skein/skein_block.c
> > +++ b/drivers/staging/skein/skein_block.c
> > @@ -82,10 +82,10 @@ do {
> >                   \
> >  } while (0)
> >  #else
> >  /* looping version */
> > -#define R256(p0, p1, p2, p3, ROT, r_num) \
>
> Why did you not fix the definition of R256 in the other half of
> the "#if SKEIN_UNROLL_256 == 0" code?  Or the definitions for R512
> and R1024?  You cleaned up one case, and left another 5 cases alone....
>
> (This is part of why blindly fixing checkpatch complaints without actually
> looking at the code and understanding isn't always a good thing)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150108/1b171d6d/attachment.html 

Submitted a first patch and no reply

[Valdis.Kletnieks at vt.edu]

On Thu, 08 Jan 2015 12:37:15 -0800, shirish gajera said:

> That's hwy I just fix one warning.

That means "don't fix a warning about indentation *and* a  warning
about trailing blanks in the same patch".

Also, if you're fixing a style issue, you should actually *review* the
code, and make sure you fix the issue everywhere, *even if the other ones
didn't prompt a warning*.

It's interesting that the warning only popped on the one case where the
"/* looping version */" is on a separate line, not on the #else line.

Am cc'ing Joe Perches for that question. Joe - to get you up to speed,
checkpatch was run against drivers/staging/skein/skein_block.c, and it
flagged one definition of the macro R256, but not the other one, or the
two definitions each for R512 and R1024.  Any idea why?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150108/aee60739/attachment.bin 

Submitted a first patch and no reply

[Joe Perches]

On Thu, 2015-01-08 at 15:49 -0500, Valdis.Kletnieks at vt.edu wrote:
> On Thu, 08 Jan 2015 12:37:15 -0800, shirish gajera said:
> 
> > That's hwy I just fix one warning.
> 
> That means "don't fix a warning about indentation *and* a  warning
> about trailing blanks in the same patch".
> 
> Also, if you're fixing a style issue, you should actually *review* the
> code, and make sure you fix the issue everywhere, *even if the other ones
> didn't prompt a warning*.
> 
> It's interesting that the warning only popped on the one case where the
> "/* looping version */" is on a separate line, not on the #else line.
> 
> Am cc'ing Joe Perches for that question. Joe - to get you up to speed,
> checkpatch was run against drivers/staging/skein/skein_block.c, and it
> flagged one definition of the macro R256, but not the other one, or the
> two definitions each for R512 and R1024.  Any idea why?

I believe the comment in the macro interferes with
the perl regex.

        #if SKEIN_UNROLL_256 == 0
        #define R256(p0, p1, p2, p3, ROT, r_num) /* fully unrolled */ \
        do {                                                          \
        	ROUND256(p0, p1, p2, p3, ROT, r_num);                 \
        } while (0)

With the latest version in -next I get:

$ ./scripts/checkpatch.pl -f --strict drivers/staging/skein/skein_block.c --types=SINGLE_STATEMENT_DO_WHILE_MACRO
WARNING: Single statement macros should not use a do {} while (0) loop
#85: FILE: drivers/staging/skein/skein_block.c:85:
+#define R256(p0, p1, p2, p3, ROT, r_num) \
+do { \
+	ROUND256(p0, p1, p2, p3, ROT, r_num); \
+} while (0)

WARNING: Single statement macros should not use a do {} while (0) loop
#176: FILE: drivers/staging/skein/skein_block.c:176:
+#define R512(p0, p1, p2, p3, p4, p5, p6, p7, ROT, r_num)                 \
+do {                                                                     \
+	ROUND512(p0, p1, p2, p3, p4, p5, p6, p7, ROT, r_num);            \
+} while (0)

WARNING: Single statement macros should not use a do {} while (0) loop
#264: FILE: drivers/staging/skein/skein_block.c:264:
+#define R1024(p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, pA, pB, pC, pD, pE, pF, \
+	      ROT, rn)                                                        \
+do {                                                                          \
+	ROUND1024(p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, pA, pB, pC, pD, pE, \
+		  pF, ROT, rn);                                               \
+} while (0)

WARNING: Single statement macros should not use a do {} while (0) loop
#292: FILE: drivers/staging/skein/skein_block.c:292:
+#define R1024(p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, pA, pB, pC, pD, pE, pF, \
+	      ROT, rn)                                                        \
+do {                                                                          \
+	ROUND1024(p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, pA, pB, pC, pD, pE, \
+		  pF, ROT, rn);                                               \
+} while (0)

total: 0 errors, 4 warnings, 0 checks, 795 lines checked

NOTE: Used message types: SINGLE_STATEMENT_DO_WHILE_MACRO

drivers/staging/skein/skein_block.c has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

Submitted a first patch and no reply

[shirish gajera]

I will make necessary and try to re- submit the patch.


On Thu, Jan 8, 2015 at 1:14 PM, Joe Perches <joe@perches.com> wrote:

> On Thu, 2015-01-08 at 15:49 -0500, Valdis.Kletnieks at vt.edu wrote:
> > On Thu, 08 Jan 2015 12:37:15 -0800, shirish gajera said:
> >
> > > That's hwy I just fix one warning.
> >
> > That means "don't fix a warning about indentation *and* a  warning
> > about trailing blanks in the same patch".
> >
> > Also, if you're fixing a style issue, you should actually *review* the
> > code, and make sure you fix the issue everywhere, *even if the other ones
> > didn't prompt a warning*.
> >
> > It's interesting that the warning only popped on the one case where the
> > "/* looping version */" is on a separate line, not on the #else line.
> >
> > Am cc'ing Joe Perches for that question. Joe - to get you up to speed,
> > checkpatch was run against drivers/staging/skein/skein_block.c, and it
> > flagged one definition of the macro R256, but not the other one, or the
> > two definitions each for R512 and R1024.  Any idea why?
>
> I believe the comment in the macro interferes with
> the perl regex.
>
>         #if SKEIN_UNROLL_256 == 0
>         #define R256(p0, p1, p2, p3, ROT, r_num) /* fully unrolled */ \
>         do {                                                          \
>                 ROUND256(p0, p1, p2, p3, ROT, r_num);                 \
>         } while (0)
>
> With the latest version in -next I get:
>
> $ ./scripts/checkpatch.pl -f --strict drivers/staging/skein/skein_block.c
> --types=SINGLE_STATEMENT_DO_WHILE_MACRO
> WARNING: Single statement macros should not use a do {} while (0) loop
> #85: FILE: drivers/staging/skein/skein_block.c:85:
> +#define R256(p0, p1, p2, p3, ROT, r_num) \
> +do { \
> +       ROUND256(p0, p1, p2, p3, ROT, r_num); \
> +} while (0)
>
> WARNING: Single statement macros should not use a do {} while (0) loop
> #176: FILE: drivers/staging/skein/skein_block.c:176:
> +#define R512(p0, p1, p2, p3, p4, p5, p6, p7, ROT, r_num)                 \
> +do {                                                                     \
> +       ROUND512(p0, p1, p2, p3, p4, p5, p6, p7, ROT, r_num);            \
> +} while (0)
>
> WARNING: Single statement macros should not use a do {} while (0) loop
> #264: FILE: drivers/staging/skein/skein_block.c:264:
> +#define R1024(p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, pA, pB, pC, pD, pE,
> pF, \
> +             ROT, rn)
>     \
> +do {
>     \
> +       ROUND1024(p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, pA, pB, pC, pD,
> pE, \
> +                 pF, ROT, rn);
>    \
> +} while (0)
>
> WARNING: Single statement macros should not use a do {} while (0) loop
> #292: FILE: drivers/staging/skein/skein_block.c:292:
> +#define R1024(p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, pA, pB, pC, pD, pE,
> pF, \
> +             ROT, rn)
>     \
> +do {
>     \
> +       ROUND1024(p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, pA, pB, pC, pD,
> pE, \
> +                 pF, ROT, rn);
>    \
> +} while (0)
>
> total: 0 errors, 4 warnings, 0 checks, 795 lines checked
>
> NOTE: Used message types: SINGLE_STATEMENT_DO_WHILE_MACRO
>
> drivers/staging/skein/skein_block.c has style problems, please review.
>
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150108/6b72e29e/attachment-0001.html 

[PATCH] checkpatch: Allow comments in macros tested for single statements

[Joe Perches]

Convert all the comments to spaces before testing for
single statement macros.

Reported-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
Signed-off-by: Joe Perches <joe@perches.com>
---
> It's interesting that the warning only popped on the one case where the
> "/* looping version */" is on a separate line, not on the #else line.
> 
> Am cc'ing Joe Perches for that question. Joe - to get you up to speed,
> checkpatch was run against drivers/staging/skein/skein_block.c, and it
> flagged one definition of the macro R256, but not the other one, or the
> two definitions each for R512 and R1024.  Any idea why?

Yup, that was it...

 scripts/checkpatch.pl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6afc24b..6ac355e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4260,6 +4260,7 @@ sub process {
 			$ctx = $dstat;
 
 			$dstat =~ s/\\\n.//g;
+			$dstat =~ s/$;/ /g;
 
 			if ($dstat =~ /^\+\s*#\s*define\s+$Ident\s*${balanced_parens}\s*do\s*{(.*)\s*}\s*while\s*\(\s*0\s*\)\s*([;\s]*)\s*$/) {
 				my $stmts = $2;

Submitted a first patch and no reply

[michi1 at michaelblizek.twilightparadox.com]

Hi!

On 12:00 Thu 08 Jan     , shirish gajera wrote:
> This patch fixes the checkpatch.pl warning:
> 
> WARNING: Single statement macros should not use a do {} while (0) loop
> 
> I have added single statement in curly braces, because it was giving
> me "WARNING: macros should not use a trailing semicolon".
...
> -#define R256(p0, p1, p2, p3, ROT, r_num) \
> -do { \
> -       ROUND256(p0, p1, p2, p3, ROT, r_num); \
> -} while (0)
> +#define R256(p0, p1, p2, p3, ROT, r_num)        \
> +{                                              \
> +       ROUND256(p0, p1, p2, p3, ROT, r_num);   \
> +}

This is wrong. Take a look at the reason of the do...while:
http://gcc.gnu.org/onlinedocs/cpp/Swallowing-the-Semicolon.html


You probably want to replace it with something like this:
#define R256(p0, p1, p2, p3, ROT, r_num) ROUND256(p0, p1, p2, p3, ROT, r_num)
or maybe even convert it to an inline function.

	-Michi
-- 
programing a layer 3+4 network protocol for mesh networks
see http://michaelblizek.twilightparadox.com