Solving checkpatch error with lex

Solving checkpatch error with lex

[karthik nayak]

Hello,
was trying to fix the checkpatch error of not using c99 comments in the 
file
drivers/staging/rtl8192u/r8192U_dm.c
so I wrote a lex program to do so since there were way too many to do it 
manually.
Could anyone have a look at my lex code and tell me if I need to change it.
I have attached the lex code.
As per my testing it works perfectly, would love a second opinion.
Thanks,
Karthik
-------------- next part --------------
%{
#include <stdio.h>	
%}

%%
"//".* {fprintf(yyout, "/*%s*/", yytext + 2);}
(.|\n) {fprintf(yyout, "%s", yytext);}
%%

main()
{
	yyin = fopen("tmp.c", "r");
	yyout = fopen("aaa.c", "w");
	yylex();
	fclose(yyin);
	fclose(yyout);
}

yywrap()
{
	return 1;
}

Solving checkpatch error with lex

[Valdis.Kletnieks at vt.edu]

On Thu, 18 Dec 2014 00:04:48 +0530, karthik nayak said:
> "//".* {fprintf(yyout, "/*%s*/", yytext + 2);}

This fails to fix block comments:

// this is line one
// this is line two
// this is line three

to
/*
 * THis is line one
 * This is line two
 * this is line three
 */
-------------- 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/20141217/defaafbd/attachment.bin 

Solving checkpatch error with lex

[Paul Bolle]

On Thu, 2014-12-18 at 00:04 +0530, karthik nayak wrote:
> was trying to fix the checkpatch error of not using c99 comments in the 
> file
> drivers/staging/rtl8192u/r8192U_dm.c
> so I wrote a lex program to do so since there were way too many to do it 
> manually.
> Could anyone have a look at my lex code and tell me if I need to change it.
> I have attached the lex code.
> As per my testing it works perfectly, would love a second opinion.

I see no reason to review someone's lex code. I guess very few people
are actually willing to do that.

Why don't you just send in the diff? Or even better, the patch you
intend to submit. Almost everybody here can read diffs or patches.

Hope this helps,


Paul Bolle

Solving checkpatch error with lex

[karthik nayak]

 On Thu, Dec 18, 2014, 12:37 AM Paul Bolle <pebolle@tiscali.nl> wrote:

On Thu, 2014-12-18 at 00:04 +0530, karthik nayak wrote:
> was trying to fix the checkpatch error of not using c99 comments in the
> file
> drivers/staging/rtl8192u/r8192U_dm.c
> so I wrote a lex program to do so since there were way too many to do it
> manually.
> Could anyone have a look at my lex code and tell me if I need to change
it.
> I have attached the lex code.
> As per my testing it works perfectly, would love a second opinion.

I see no reason to review someone's lex code. I guess very few people
are actually willing to do that.

Why don't you just send in the diff? Or even better, the patch you
intend to submit. Almost everybody here can read diffs or patches.

Hope this helps,

Paul Bolle



 Hello Paul,
The only reason I didn't do that is cause the file is too large to manually
have a look at, even the diff tends to be very huge.
But if you think that would help, I wouldn't mind attaching the diff.
Thanks
Karthik
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20141217/34c4c33a/attachment.html 

Solving checkpatch error with lex

[Greg KH]

On Wed, Dec 17, 2014 at 07:10:24PM +0000, karthik nayak wrote:
> 
> On Thu, Dec 18, 2014, 12:37 AM?Paul Bolle <pebolle@tiscali.nl> wrote:
> 
>     On Thu, 2014-12-18 at 00:04 +0530, karthik nayak wrote:
>     > was trying to fix the checkpatch error of not using c99 comments in the
>     > file
>     > drivers/staging/rtl8192u/r8192U_dm.c
>     > so I wrote a lex program to do so since there were way too many to do it
>     > manually.
>     > Could anyone have a look at my lex code and tell me if I need to change
>     it.
>     > I have attached the lex code.
>     > As per my testing it works perfectly, would love a second opinion.
> 
>     I see no reason to review someone's lex code. I guess very few people
>     are actually willing to do that.
> 
>     Why don't you just send in the diff? Or even better, the patch you
>     intend to submit. Almost everybody here can read diffs or patches.
> 
>     Hope this helps,
> 
>     Paul Bolle
> 
> 
> 
> 
> Hello Paul,
> The only reason I didn't do that is cause the file is too large to manually
> have a look at, even the diff tends to be very huge.

Then the diff will not be accepted upstream either if it is too big to
review.

Solving checkpatch error with lex

[Valdis.Kletnieks at vt.edu]

On Wed, 17 Dec 2014 19:10:24 +0000, karthik nayak said:

> The only reason I didn't do that is cause the file is too large to
> manually have a look at, even the diff tends to be very huge.

[/usr/src/linux-next] grep '//' drivers/staging/rtl8192u/r8192U_dm.c | wc -l
373

And why use lex when sed is up to the task?

 [/usr/src/linux-next] sed 's?//(.*)$?/* \1 */?' drivers/staging/rtl8192u/r8192U_dm.c >| /tmp/z99
sed: -e expression #1, char 19: invalid reference \1 on `s' command's RHS
1 [/usr/src/linux-next] sed 's?//\(.*\)$?/* \1 */?' drivers/staging/rtl8192u/r8192U_dm.c >| /tmp/z99
0 [/usr/src/linux-next] diff -u drivers/staging/rtl8192u/r8192U_dm.c /tmp/z99 | head -20
--- drivers/staging/rtl8192u/r8192U_dm.c        2014-11-20 16:18:35.941071156 -0500
+++ /tmp/z99    2014-12-17 16:22:42.831566630 -0500
@@ -21,9 +21,9 @@
 #include "r8190_rtl8256.h"
 #include "r819xU_cmdpkt.h"
 /*---------------------------Define Local Constant---------------------------*/
-//
-// Indicate different AP vendor for IOT issue.
-//
+/*  */
+/*  Indicate different AP vendor for IOT issue. */
+/*  */
 static u32 edca_setting_DL[HT_IOT_PEER_MAX] =
                { 0x5e4322,     0x5e4322,       0x5e4322,       0x604322,       0xa44f,         0x5ea44f};
 static u32 edca_setting_UL[HT_IOT_PEER_MAX] =
@@ -36,11 +36,11 @@


 /*------------------------Define global variable-----------------------------*/
-// Debug variable ?
0 [/usr/src/linux-next] diff -u drivers/staging/rtl8192u/r8192U_dm.c /tmp/z99 | wc -l
1960

2K lines isn't that hard to review, if the *only* thing you do is fix c99
comments.  I'd recommend it as *two* patches - one to do basically what your
lex code does and convert '// foo bar baz' to '/* foo bar baz */'

And then a *second* patch to fix block comments to   /*    *     */ form.

-------------- 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/20141217/44410b83/attachment.bin 

Solving checkpatch error with lex

[Greg KH]

On Wed, Dec 17, 2014 at 04:24:15PM -0500, Valdis.Kletnieks at vt.edu wrote:
> On Wed, 17 Dec 2014 19:10:24 +0000, karthik nayak said:
> 
> > The only reason I didn't do that is cause the file is too large to
> > manually have a look at, even the diff tends to be very huge.
> 
> [/usr/src/linux-next] grep '//' drivers/staging/rtl8192u/r8192U_dm.c | wc -l
> 373
> 
> And why use lex when sed is up to the task?
> 
>  [/usr/src/linux-next] sed 's?//(.*)$?/* \1 */?' drivers/staging/rtl8192u/r8192U_dm.c >| /tmp/z99
> sed: -e expression #1, char 19: invalid reference \1 on `s' command's RHS
> 1 [/usr/src/linux-next] sed 's?//\(.*\)$?/* \1 */?' drivers/staging/rtl8192u/r8192U_dm.c >| /tmp/z99
> 0 [/usr/src/linux-next] diff -u drivers/staging/rtl8192u/r8192U_dm.c /tmp/z99 | head -20
> --- drivers/staging/rtl8192u/r8192U_dm.c        2014-11-20 16:18:35.941071156 -0500
> +++ /tmp/z99    2014-12-17 16:22:42.831566630 -0500
> @@ -21,9 +21,9 @@
>  #include "r8190_rtl8256.h"
>  #include "r819xU_cmdpkt.h"
>  /*---------------------------Define Local Constant---------------------------*/
> -//
> -// Indicate different AP vendor for IOT issue.
> -//
> +/*  */
> +/*  Indicate different AP vendor for IOT issue. */
> +/*  */
>  static u32 edca_setting_DL[HT_IOT_PEER_MAX] =
>                 { 0x5e4322,     0x5e4322,       0x5e4322,       0x604322,       0xa44f,         0x5ea44f};
>  static u32 edca_setting_UL[HT_IOT_PEER_MAX] =
> @@ -36,11 +36,11 @@
> 
> 
>  /*------------------------Define global variable-----------------------------*/
> -// Debug variable ?
> 0 [/usr/src/linux-next] diff -u drivers/staging/rtl8192u/r8192U_dm.c /tmp/z99 | wc -l
> 1960

What a mess, I'll gladly reject a patch that does this.

Look at that multi-line crud.

ick ick ick, don't run auto-formatting tools just to fix up checkpatch
issues.  If that was all that is needed, I would have done that a long
time ago, that's not the goal here people...

greg k-h

Solving checkpatch error with lex

[karthik nayak]

Thanks for your input.

On Thu, Dec 18, 2014, 6:17 AM Greg KH <greg@kroah.com> wrote:

> On Wed, Dec 17, 2014 at 04:24:15PM -0500, Valdis.Kletnieks at vt.edu wrote:
> > On Wed, 17 Dec 2014 19:10:24 +0000, karthik nayak said:
> >
> > > The only reason I didn't do that is cause the file is too large to
> > > manually have a look at, even the diff tends to be very huge.
> >
> > [/usr/src/linux-next] grep '//' drivers/staging/rtl8192u/r8192U_dm.c |
> wc -l
> > 373
> >
> > And why use lex when sed is up to the task?
> >
> >  [/usr/src/linux-next] sed 's?//(.*)$?/* \1 */?'
> drivers/staging/rtl8192u/r8192U_dm.c >| /tmp/z99
> > sed: -e expression #1, char 19: invalid reference \1 on `s' command's RHS
> > 1 [/usr/src/linux-next] sed 's?//\(.*\)$?/* \1 */?'
> drivers/staging/rtl8192u/r8192U_dm.c >| /tmp/z99
> > 0 [/usr/src/linux-next] diff -u drivers/staging/rtl8192u/r8192U_dm.c
> /tmp/z99 | head -20
> > --- drivers/staging/rtl8192u/r8192U_dm.c        2014-11-20
> 16:18:35.941071156 -0500
> > +++ /tmp/z99    2014-12-17 16:22:42.831566630 -0500
> > @@ -21,9 +21,9 @@
> >  #include "r8190_rtl8256.h"
> >  #include "r819xU_cmdpkt.h"
> >  /*---------------------------Define Local
> Constant---------------------------*/
> > -//
> > -// Indicate different AP vendor for IOT issue.
> > -//
> > +/*  */
> > +/*  Indicate different AP vendor for IOT issue. */
> > +/*  */
> >  static u32 edca_setting_DL[HT_IOT_PEER_MAX] =
> >                 { 0x5e4322,     0x5e4322,       0x5e4322,
>  0x604322,       0xa44f,         0x5ea44f};
> >  static u32 edca_setting_UL[HT_IOT_PEER_MAX] =
> > @@ -36,11 +36,11 @@
> >
> >
> >  /*------------------------Define global variable----------------------
> -------*/
> > -// Debug variable ?
> > 0 [/usr/src/linux-next] diff -u drivers/staging/rtl8192u/r8192U_dm.c
> /tmp/z99 | wc -l
> > 1960
>
> What a mess, I'll gladly reject a patch that does this.
>
> Look at that multi-line crud.
>
> ick ick ick, don't run auto-formatting tools just to fix up checkpatch
> issues.  If that was all that is needed, I would have done that a long
> time ago, that's not the goal here people...
>
> greg k-h
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20141218/b37600be/attachment.html 

Solving checkpatch error with lex

[Paul Bolle]

On Wed, 2014-12-17 at 20:07 +0100, Paul Bolle wrote:
> I see no reason to review someone's lex code. I guess very few people
> are actually willing to do that.

My statement is not incorrect just because Valdis did actually review
your code!


Paul Bolle