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

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

[Joe Eloff]

>From b7f34cd4bf14c07f64a95f70410f34e2c843fd3b Mon Sep 17 00:00:00 2001
From: Joe Eloff <kagen101@gmail.com>
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 <kagen101@gmail.com>

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

diff --git a/drivers/staging/cx25821/cx25821-audio-upstream.c b/drivers/staging/cx25821/cx25821-audio-upstream.c
index eb39d13..e7346ec 100644
--- a/drivers/staging/cx25821/cx25821-audio-upstream.c
+++ b/drivers/staging/cx25821/cx25821-audio-upstream.c
@@ -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);
-- 
1.6.3.3

Re: [PATCH] Staging: cx25821: fix brace coding style issue in cx25821-audio-upstream.c

[Aldo Cedillo]

> 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 <kagen101@gmail.com>
>
> ---
>  drivers/staging/cx25821/cx25821-audio-upstream.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/cx25821/cx25821-audio-upstream.c b/drivers/staging/cx25821/cx25821-audio-upstream.c
> index eb39d13..e7346ec 100644
> --- a/drivers/staging/cx25821/cx25821-audio-upstream.c
> +++ b/drivers/staging/cx25821/cx25821-audio-upstream.c
> @@ -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.

>                /* 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.
> +
>        } else {
So the first brace closes the brace of the if line.
>                str_length = strlen(_defaultAudioName);
>                dev->_audiofilename = kmalloc(str_length + 1, GFP_KERNEL);

Thanks,
Aldo Brett

Re: [PATCH] Staging: cx25821: fix brace coding style issue in cx25821-audio-upstream.c

[Joe Eloff]

On Tue, 2010-07-06 at 12:12 -0500, Aldo Cedillo wrote:
> > 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 <kagen101@gmail.com>
> >
> > ---
> >  drivers/staging/cx25821/cx25821-audio-upstream.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/cx25821/cx25821-audio-upstream.c b/drivers/staging/cx25821/cx25821-audio-upstream.c
> > index eb39d13..e7346ec 100644
> > --- a/drivers/staging/cx25821/cx25821-audio-upstream.c
> > +++ b/drivers/staging/cx25821/cx25821-audio-upstream.c
> > @@ -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.
> 
> >                /* 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.
> > +
> >        } else {
> So the first brace closes the brace of the if line.
> >                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

Re: [PATCH] Staging: cx25821: fix brace coding style issue in cx25821-audio-upstream.c

[Mauro Carvalho Chehab]

Em 06-07-2010 14:24, Joe Eloff escreveu:
> On Tue, 2010-07-06 at 12:12 -0500, Aldo Cedillo wrote:
>>> 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 <kagen101@gmail.com>
>>>
>>> ---
>>>  drivers/staging/cx25821/cx25821-audio-upstream.c |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/cx25821/cx25821-audio-upstream.c b/drivers/staging/cx25821/cx25821-audio-upstream.c
>>> index eb39d13..e7346ec 100644
>>> --- a/drivers/staging/cx25821/cx25821-audio-upstream.c
>>> +++ b/drivers/staging/cx25821/cx25821-audio-upstream.c
>>> @@ -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.
>>
>>>                /* 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.
>>> +
>>>        } else {
>> So the first brace closes the brace of the if line.
>>>                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 

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.