Staging: cx25821: fix brace coding style issue in cx25821-audio-upstream.c

PATCH In review Joe Eloff 3 participants 4 messages Jul 4, 2010 – Jul 6, 2010 Staging

lore

Share & tools

lore

Joe Eloff

author

2010-07-04 10:09 UTC #0 lore raw

PATCH 1/1 Staging: cx25821: fix brace coding style issue in cx25821-audio-upstream.c

Quoted 1 line

From b7f34cd4bf14c07f64a95f70410f34e2c843fd3b Mon Sep 17 00:00:00 2001

From: Joe Eloff <[email protected]>
Date: Sun, 4 Jul 2010 11:37:03 +0200
Subject: [PATCH] Staging: cx25821: fix brace coding style issue in cx25821-audio-upstream.c
This is a patch to the cx25821.c file that fixes up a brace warning found by the checkpatch.pl tool

Signed-off-by Joe Eloff <[email protected]>

---
 drivers/staging/cx25821/cx25821-audio-upstream.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/staging/cx25821/cx25821-audio-upstream.c +2 -2

index eb39d13..e7346ec 100644
@@ -762,9 +762,9 @@ int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select)
 		       str_length + 1);
 
 		/* Default if filename is empty string */
-		if (strcmp(dev->input_audiofilename, "") == 0) {
+		if (strcmp(dev->input_audiofilename, "") == 0)
 			dev->_audiofilename = "/root/audioGOOD.wav";
-		}
+
 	} else {
 		str_length = strlen(_defaultAudioName);
 		dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);

Signature · 1 line

1.6.3.3

Aldo Cedillo

2010-07-06 17:12 UTC #1 lore raw

Quoted 13 lines

Subject: [PATCH] Staging: cx25821: fix brace coding style issue in cx25821-audio-upstream.c This is a patch to the cx25821.c file that fixes up a brace warning found by the checkpatch.pl tool
Signed-off-by Joe Eloff <[email protected]>
---
drivers/staging/cx25821/cx25821-audio-upstream.c | 4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/staging/cx25821/cx25821-audio-upstream.c +0 -0
index eb39d13..e7346ec 100644

drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 762

Quoted 1 line

drivers/staging/cx25821/cx25821-audio-upstream.c +0 -0

@@ -762,9 +762,9 @@ int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select)
                       str_length + 1);

I haven't tried checkpatch.pl over that file, but in the CodingStyle document you can read this:

Do not unnecessarily use braces where a single statement will do.

if (condition)

        action();

This does not apply if one branch of a conditional statement is a single statement. Use braces in both branches.

if (condition) {
        do_this();
        do_that();
} else {
        otherwise();
}

Which one is the correct style? I ask these because I have seen this in other parts of the kernel. So maybe I can help to homogenize this.

drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 767

Quoted 5 lines

drivers/staging/cx25821/cx25821-audio-upstream.c +1 -2

                /* Default if filename is empty string */
-               if (strcmp(dev->input_audiofilename, "") == 0) {
+               if (strcmp(dev->input_audiofilename, "") == 0)
                        dev->_audiofilename = "/root/audioGOOD.wav";
-               }

I believe only this one has to go.

drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 768

Quoted 2 lines

drivers/staging/cx25821/cx25821-audio-upstream.c +1 -0
+
        } else {

So the first brace closes the brace of the if line.

drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 770

Quoted 2 lines

drivers/staging/cx25821/cx25821-audio-upstream.c +0 -0

                str_length = strlen(_defaultAudioName);
                dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);

Thanks,
Aldo Brett

Joe Eloff

author

2010-07-06 18:31 UTC #2 lore raw

Quoted 43 lines · Aldo Cedillo

Quoted 13 lines
Subject: [PATCH] Staging: cx25821: fix brace coding style issue in cx25821-audio-upstream.c
This is a patch to the cx25821.c file that fixes up a brace warning found by the checkpatch.pl tool
Signed-off-by Joe Eloff <[email protected]>
---
 drivers/staging/cx25821/cx25821-audio-upstream.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/staging/cx25821/cx25821-audio-upstream.c +0 -0
index eb39d13..e7346ec 100644
drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 762
Quoted 1 line
drivers/staging/cx25821/cx25821-audio-upstream.c +0 -0
@@ -762,9 +762,9 @@ int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select)
                      str_length + 1);
I haven't tried checkpatch.pl over that file, but in the CodingStyle document you can read this:
Do not unnecessarily use braces where a single statement will do.
if (condition)
        action();
This does not apply if one branch of a conditional statement is a single statement. Use braces in both branches.
if (condition) {
        do_this();
        do_that();
} else {
        otherwise();
}
Which one is the correct style? I ask these because I have seen this in other parts of the kernel. So maybe I can help to homogenize this.
drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 767
Quoted 5 lines
drivers/staging/cx25821/cx25821-audio-upstream.c +1 -2
               /* Default if filename is empty string */
-               if (strcmp(dev->input_audiofilename, "") == 0) {
+               if (strcmp(dev->input_audiofilename, "") == 0)
                       dev->_audiofilename = "/root/audioGOOD.wav";
-               }
I believe only this one has to go.

Quoted 8 lines

drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 768
Quoted 2 lines
drivers/staging/cx25821/cx25821-audio-upstream.c +1 -0
+
       } else {
So the first brace closes the brace of the if line.
drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 770
Quoted 2 lines
drivers/staging/cx25821/cx25821-audio-upstream.c +0 -0
               str_length = strlen(_defaultAudioName);
               dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);
Thanks,
Aldo Brett

Thats good to know thanks for pointing that out, I have picked up a few discrepancies with interpretation and checkpatch.pl.
Am noting it and trying to patch checkpatch.pl as I continue :).

I think checkpatch.pl points this out as the opposite to interpretation thou otherwise I would not have patched it I guess.

Current TODO on checkpatch:

1. type i = 0 ; /* space before ; is not picked up by checkpatch.
2. What you mentioned is interpreted different by checkpatch.pl

Regards,

Joe

Mauro Carvalho Chehab

2010-07-06 17:37 UTC #3 lore raw

Em 06-07-2010 14:24, Joe Eloff escreveu:

Quoted 57 lines · Aldo Cedillo

Quoted 43 lines
Quoted 13 lines
Subject: [PATCH] Staging: cx25821: fix brace coding style issue in cx25821-audio-upstream.c
This is a patch to the cx25821.c file that fixes up a brace warning found by the checkpatch.pl tool
Signed-off-by Joe Eloff <[email protected]>
---
 drivers/staging/cx25821/cx25821-audio-upstream.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/staging/cx25821/cx25821-audio-upstream.c +0 -0
index eb39d13..e7346ec 100644
drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 762
Quoted 1 line
drivers/staging/cx25821/cx25821-audio-upstream.c +0 -0
@@ -762,9 +762,9 @@ int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select)
                      str_length + 1);
I haven't tried checkpatch.pl over that file, but in the CodingStyle document you can read this:
Do not unnecessarily use braces where a single statement will do.
if (condition)
        action();
This does not apply if one branch of a conditional statement is a single statement. Use braces in both branches.
if (condition) {
        do_this();
        do_that();
} else {
        otherwise();
}
Which one is the correct style? I ask these because I have seen this in other parts of the kernel. So maybe I can help to homogenize this.
drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 767
Quoted 5 lines
drivers/staging/cx25821/cx25821-audio-upstream.c +1 -2
               /* Default if filename is empty string */
-               if (strcmp(dev->input_audiofilename, "") == 0) {
+               if (strcmp(dev->input_audiofilename, "") == 0)
                       dev->_audiofilename = "/root/audioGOOD.wav";
-               }
I believe only this one has to go.
Quoted 8 lines
drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 768
Quoted 2 lines
drivers/staging/cx25821/cx25821-audio-upstream.c +1 -0
+
       } else {
So the first brace closes the brace of the if line.
drivers/staging/cx25821/cx25821-audio-upstream.c int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) · line 770
Quoted 2 lines
drivers/staging/cx25821/cx25821-audio-upstream.c +0 -0
               str_length = strlen(_defaultAudioName);
               dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);
Thanks,
Aldo Brett
Thats good to know thanks for pointing that out, I have picked up a few discrepancies with interpretation and checkpatch.pl.
Am noting it and trying to patch checkpatch.pl as I continue :).

Quoted 7 lines

I think checkpatch.pl points this out as the opposite to interpretation thou otherwise I would not have patched it I guess.
Current TODO on checkpatch:
1. type i = 0 ; /* space before ; is not picked up by checkpatch.
2. What you mentioned is interpreted different by checkpatch.pl

If you intend to submit more patches to cx25821, please generate it against my tree, as the driver suffered a major cleanup recently.

	http://git.linuxtv.org/v4l-dvb.git
	branch devel/for_v2.6.36

Cheers,
Mauro.