[PATCH] Staging: speakup: synth.c: removed a space

[PATCH] Staging: speakup: synth.c: removed a space

[Aldo Iljazi]

Line 468: Removed a space before a semicolon.

Signed-off-by: Aldo Iljazi <mail@aldo.io>
---
 drivers/staging/speakup/synth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/synth.c b/drivers/staging/speakup/synth.c
index 0b3549b..2a4b348 100644
--- a/drivers/staging/speakup/synth.c
+++ b/drivers/staging/speakup/synth.c
@@ -465,7 +465,7 @@ void synth_remove(struct spk_synth *in_synth)
 		if (in_synth == synths[i])
 			break;
 	}
-	for ( ; synths[i] != NULL; i++) /* compress table */
+	for (; synths[i] != NULL; i++) /* compress table */
 		synths[i] = synths[i+1];
 	module_status = 0;
 	mutex_unlock(&spk_mutex);
-- 
1.8.3.2

Re: [PATCH] Staging: speakup: synth.c: removed a space

[Samuel Thibault]

Aldo Iljazi, le Tue 03 Dec 2013 22:08:03 +0200, a écrit :
> Line 468: Removed a space before a semicolon.

Err, I'd rather make it really visible that the for loop doesn't have
its first statement?

Samuel

Re: [PATCH] Staging: speakup: synth.c: removed a space

[Aldo Iljazi]

 Samuel Thibault wrote:

> Err, I'd rather make it really visible that the for loop doesn't have
> its first statement?

Wouldn't it be better if you add a comment there? So it would follow the
coding style?
-- 
Aldo Iljazi

Re: [PATCH] Staging: speakup: synth.c: removed a space

[Dan Carpenter]

On Wed, Dec 04, 2013 at 06:35:15AM +0200, Aldo Iljazi wrote:
>  Samuel Thibault wrote:
> 
> > Err, I'd rather make it really visible that the for loop doesn't have
> > its first statement?
> 
> Wouldn't it be better if you add a comment there? So it would follow the
> coding style?

No.  Adding obvious comments is more annoying than the space.

This seems like a small bug in checkpatch.pl.  Joe, the problem is this
code:

	for ( ; x; x++) {

It's complaining about the space character before the semicolon.

regards,
dan carpenter

Re: [PATCH] Staging: speakup: synth.c: removed a space

[Joe Perches]

On Wed, 2013-12-04 at 10:38 +0300, Dan Carpenter wrote:
> On Wed, Dec 04, 2013 at 06:35:15AM +0200, Aldo Iljazi wrote:
> >  Samuel Thibault wrote:
> > 
> > > Err, I'd rather make it really visible that the for loop doesn't have
> > > its first statement?
> > 
> > Wouldn't it be better if you add a comment there? So it would follow the
> > coding style?
> 
> No.  Adding obvious comments is more annoying than the space.
> 
> This seems like a small bug in checkpatch.pl.  Joe, the problem is this
> code:
> 
> 	for ( ; x; x++) {
> 
> It's complaining about the space character before the semicolon.

Shrug.  checkpatch isn't and can't be perfect.

I think it's for things like

	for (;;)

and that's better than

	for ( ; ; )
or
	for ( ;; )
or
	for ( ;;)

Re: [PATCH] Staging: speakup: synth.c: removed a space

[Dan Carpenter]

You and I generally agree on style preferences...  I think the warning
should be limited to grep " ;$".

I did a grep on the kernel for ' ;' and found 8000 results.  6000 of
them are caught by my semicolon before the newline rule.  The remaining
2000 are assembly, macros, and crappy for loops.

regards,
dan carpenter

[PATCH -next] checkpatch: Warn only on "space before semicolon" at end of line

[Joe Perches]

The "space before a non-naked semicolon" test has
unwanted output when used in "for ( ;; )" loops.

Make the test work only on end-of-line statement
termination semicolons.

Signed-off-by: Joe Perches <joe@perches.com>
---
On Wed, 2013-12-04 at 11:21 +0300, Dan Carpenter wrote:
> You and I generally agree on style preferences...

True, and I think that's a good thing.

> I think the warning
> should be limited to grep " ;$".

Look at the output of:

$ git grep " ;" -- "*.[ch]"  | grep -w for
...

The "| wc -l" output above in -next is
1407

Which of those should not be warned on?

$ git grep " ;" -- "*.[ch]"  | grep -P "\bfor\s*\(\s+;" | wc -l
211

I think all of the uses like "for ( ; expression;  expression)"
are unbalanced and should be avoided.

<I go off and look at checkpatch output>

Oh.  checkpatch doesn't complain about spacing around
semicolon with for loops here.  This error is a separate
test that looks only for space before semicolon.

So, we agree after all.

This test should look only for space before end-of-statement
semicolons at EOL.

This has still has false positives with multi-line fors like:

	for ( ; expression ;
	     expression);

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 38be5d5..d26eac6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3131,7 +3131,7 @@ sub process {
 		}
 
 # check for whitespace before a non-naked semicolon
-		if ($line =~ /^\+.*\S\s+;/) {
+		if ($line =~ /^\+.*\S\s+;\s*$/) {
 			if (WARN("SPACING",
 				 "space prohibited before semicolon\n" . $herecurr) &&
 			    $fix) {

Re: [PATCH -next] checkpatch: Warn only on "space before semicolon" at end of line

[Dan Carpenter]

Thanks so much.  :)

regards,
dan carpenter