diff mbox series

media: av7110: av7110_av: Fix Switch and Case Same Indent Style Error

Message ID 20220225155622.585621-1-ahamedhusni73@gmail.com
State New
Headers show
Series media: av7110: av7110_av: Fix Switch and Case Same Indent Style Error | expand

Commit Message

Ahamed Husni Feb. 25, 2022, 3:56 p.m. UTC 
This patch fixes "switch and case should be at the same indent"
checkpatch error.

Signed-off-by: Husni Faiz <ahamedhusni73@gmail.com>
---
 drivers/staging/media/av7110/av7110_av.c | 30 ++++++++++++------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Comments

Hans Verkuil March 7, 2022, 7:28 a.m. UTC | #1 
Hi Husni,

Thank you for the patch.

The Subject line needs some work: either name the source ('av7110_av.c:') or
driver ('av7110:'), but not both. Also just stick to lower case, so:
"media: av7110_av.c: fix switch indentation"

That gives all the relevant information, and is a lot shorter.

On 2/25/22 16:56, Husni Faiz wrote:
> This patch fixes "switch and case should be at the same indent"
> checkpatch error.
> 
> Signed-off-by: Husni Faiz <ahamedhusni73@gmail.com>
> ---
>  drivers/staging/media/av7110/av7110_av.c | 30 ++++++++++++------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/media/av7110/av7110_av.c b/drivers/staging/media/av7110/av7110_av.c
> index 91f4866c7e59..1d42862e9669 100644
> --- a/drivers/staging/media/av7110/av7110_av.c
> +++ b/drivers/staging/media/av7110/av7110_av.c
> @@ -770,22 +770,22 @@ static void p_to_t(u8 const *buf, long int length, u16 pid, u8 *counter,
>  	if (length > 3 &&
>  	     buf[0] == 0x00 && buf[1] == 0x00 && buf[2] == 0x01)
>  		switch (buf[3]) {
> -			case PROG_STREAM_MAP:
> -			case PRIVATE_STREAM2:
> -			case PROG_STREAM_DIR:
> -			case ECM_STREAM     :
> -			case EMM_STREAM     :
> -			case PADDING_STREAM :
> -			case DSM_CC_STREAM  :
> -			case ISO13522_STREAM:
> -			case PRIVATE_STREAM1:
> -			case AUDIO_STREAM_S ... AUDIO_STREAM_E:
> -			case VIDEO_STREAM_S ... VIDEO_STREAM_E:
> -				pes_start = 1;
> -				break;
> +		case PROG_STREAM_MAP:
> +		case PRIVATE_STREAM2:
> +		case PROG_STREAM_DIR:
> +		case ECM_STREAM     :
> +		case EMM_STREAM     :
> +		case PADDING_STREAM :
> +		case DSM_CC_STREAM  :
> +		case ISO13522_STREAM:
> +		case PRIVATE_STREAM1:
> +		case AUDIO_STREAM_S ... AUDIO_STREAM_E:
> +		case VIDEO_STREAM_S ... VIDEO_STREAM_E:
> +			pes_start = 1;
> +			break;
>  
> -			default:
> -				break;
> +		default:
> +			break;
>  		}
>  
>  	while (c < length) {

Running checkpatch.pl over this patch give me:

ERROR: space prohibited before that ':' (ctx:WxE)
#40: FILE: drivers/staging/media/av7110/av7110_av.c:776:
+               case ECM_STREAM     :
                                    ^

ERROR: space prohibited before that ':' (ctx:WxE)
#41: FILE: drivers/staging/media/av7110/av7110_av.c:777:
+               case EMM_STREAM     :
                                    ^

ERROR: space prohibited before that ':' (ctx:WxE)
#42: FILE: drivers/staging/media/av7110/av7110_av.c:778:
+               case PADDING_STREAM :
                                    ^

ERROR: space prohibited before that ':' (ctx:WxE)
#43: FILE: drivers/staging/media/av7110/av7110_av.c:779:
+               case DSM_CC_STREAM  :
                                    ^
Can you fix that as well in a v2 of this patch?

Thanks!

	Hans
Ahamed Husni March 8, 2022, 12:50 p.m. UTC | #2 
Hello Hans,

On Mon, Mar 7, 2022 at 12:58 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> Hi Husni,
>
> Thank you for the patch.
>
> The Subject line needs some work: either name the source ('av7110_av.c:') or
> driver ('av7110:'), but not both. Also just stick to lower case, so:
> "media: av7110_av.c: fix switch indentation"
>
> That gives all the relevant information, and is a lot shorter.
Noted with thanks. I'll update the subject line in the V2 of the patch.

>
> On 2/25/22 16:56, Husni Faiz wrote:
> > This patch fixes "switch and case should be at the same indent"
> > checkpatch error.
> >
> > Signed-off-by: Husni Faiz <ahamedhusni73@gmail.com>
> > ---
> >  drivers/staging/media/av7110/av7110_av.c | 30 ++++++++++++------------
> >  1 file changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/staging/media/av7110/av7110_av.c b/drivers/staging/media/av7110/av7110_av.c
> > index 91f4866c7e59..1d42862e9669 100644
> > --- a/drivers/staging/media/av7110/av7110_av.c
> > +++ b/drivers/staging/media/av7110/av7110_av.c
> > @@ -770,22 +770,22 @@ static void p_to_t(u8 const *buf, long int length, u16 pid, u8 *counter,
> >       if (length > 3 &&
> >            buf[0] == 0x00 && buf[1] == 0x00 && buf[2] == 0x01)
> >               switch (buf[3]) {
> > -                     case PROG_STREAM_MAP:
> > -                     case PRIVATE_STREAM2:
> > -                     case PROG_STREAM_DIR:
> > -                     case ECM_STREAM     :
> > -                     case EMM_STREAM     :
> > -                     case PADDING_STREAM :
> > -                     case DSM_CC_STREAM  :
> > -                     case ISO13522_STREAM:
> > -                     case PRIVATE_STREAM1:
> > -                     case AUDIO_STREAM_S ... AUDIO_STREAM_E:
> > -                     case VIDEO_STREAM_S ... VIDEO_STREAM_E:
> > -                             pes_start = 1;
> > -                             break;
> > +             case PROG_STREAM_MAP:
> > +             case PRIVATE_STREAM2:
> > +             case PROG_STREAM_DIR:
> > +             case ECM_STREAM     :
> > +             case EMM_STREAM     :
> > +             case PADDING_STREAM :
> > +             case DSM_CC_STREAM  :
> > +             case ISO13522_STREAM:
> > +             case PRIVATE_STREAM1:
> > +             case AUDIO_STREAM_S ... AUDIO_STREAM_E:
> > +             case VIDEO_STREAM_S ... VIDEO_STREAM_E:
> > +                     pes_start = 1;
> > +                     break;
> >
> > -                     default:
> > -                             break;
> > +             default:
> > +                     break;
> >               }
> >
> >       while (c < length) {
>
> Running checkpatch.pl over this patch give me:
>
> ERROR: space prohibited before that ':' (ctx:WxE)
> #40: FILE: drivers/staging/media/av7110/av7110_av.c:776:
> +               case ECM_STREAM     :
>                                     ^
>
> ERROR: space prohibited before that ':' (ctx:WxE)
> #41: FILE: drivers/staging/media/av7110/av7110_av.c:777:
> +               case EMM_STREAM     :
>                                     ^
>
> ERROR: space prohibited before that ':' (ctx:WxE)
> #42: FILE: drivers/staging/media/av7110/av7110_av.c:778:
> +               case PADDING_STREAM :
>                                     ^
>
> ERROR: space prohibited before that ':' (ctx:WxE)
> #43: FILE: drivers/staging/media/av7110/av7110_av.c:779:
> +               case DSM_CC_STREAM  :
>                                     ^
> Can you fix that as well in a v2 of this patch?
It seems that these spaces are deliberately added by the author to
keep the case statements' colons aligned.
Some other lines in the file where the same approach has been taken
are [line 598,599,600,601] and [line 662, 663, 664, 665].
Should we leave these spaces as it is?

Thanks,
Husni.
Hans Verkuil March 8, 2022, 12:52 p.m. UTC | #3 
On 3/8/22 13:50, Ahamed Husni wrote:
> Hello Hans,
> 
> On Mon, Mar 7, 2022 at 12:58 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> Hi Husni,
>>
>> Thank you for the patch.
>>
>> The Subject line needs some work: either name the source ('av7110_av.c:') or
>> driver ('av7110:'), but not both. Also just stick to lower case, so:
>> "media: av7110_av.c: fix switch indentation"
>>
>> That gives all the relevant information, and is a lot shorter.
> Noted with thanks. I'll update the subject line in the V2 of the patch.
> 
>>
>> On 2/25/22 16:56, Husni Faiz wrote:
>>> This patch fixes "switch and case should be at the same indent"
>>> checkpatch error.
>>>
>>> Signed-off-by: Husni Faiz <ahamedhusni73@gmail.com>
>>> ---
>>>  drivers/staging/media/av7110/av7110_av.c | 30 ++++++++++++------------
>>>  1 file changed, 15 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/av7110/av7110_av.c b/drivers/staging/media/av7110/av7110_av.c
>>> index 91f4866c7e59..1d42862e9669 100644
>>> --- a/drivers/staging/media/av7110/av7110_av.c
>>> +++ b/drivers/staging/media/av7110/av7110_av.c
>>> @@ -770,22 +770,22 @@ static void p_to_t(u8 const *buf, long int length, u16 pid, u8 *counter,
>>>       if (length > 3 &&
>>>            buf[0] == 0x00 && buf[1] == 0x00 && buf[2] == 0x01)
>>>               switch (buf[3]) {
>>> -                     case PROG_STREAM_MAP:
>>> -                     case PRIVATE_STREAM2:
>>> -                     case PROG_STREAM_DIR:
>>> -                     case ECM_STREAM     :
>>> -                     case EMM_STREAM     :
>>> -                     case PADDING_STREAM :
>>> -                     case DSM_CC_STREAM  :
>>> -                     case ISO13522_STREAM:
>>> -                     case PRIVATE_STREAM1:
>>> -                     case AUDIO_STREAM_S ... AUDIO_STREAM_E:
>>> -                     case VIDEO_STREAM_S ... VIDEO_STREAM_E:
>>> -                             pes_start = 1;
>>> -                             break;
>>> +             case PROG_STREAM_MAP:
>>> +             case PRIVATE_STREAM2:
>>> +             case PROG_STREAM_DIR:
>>> +             case ECM_STREAM     :
>>> +             case EMM_STREAM     :
>>> +             case PADDING_STREAM :
>>> +             case DSM_CC_STREAM  :
>>> +             case ISO13522_STREAM:
>>> +             case PRIVATE_STREAM1:
>>> +             case AUDIO_STREAM_S ... AUDIO_STREAM_E:
>>> +             case VIDEO_STREAM_S ... VIDEO_STREAM_E:
>>> +                     pes_start = 1;
>>> +                     break;
>>>
>>> -                     default:
>>> -                             break;
>>> +             default:
>>> +                     break;
>>>               }
>>>
>>>       while (c < length) {
>>
>> Running checkpatch.pl over this patch give me:
>>
>> ERROR: space prohibited before that ':' (ctx:WxE)
>> #40: FILE: drivers/staging/media/av7110/av7110_av.c:776:
>> +               case ECM_STREAM     :
>>                                     ^
>>
>> ERROR: space prohibited before that ':' (ctx:WxE)
>> #41: FILE: drivers/staging/media/av7110/av7110_av.c:777:
>> +               case EMM_STREAM     :
>>                                     ^
>>
>> ERROR: space prohibited before that ':' (ctx:WxE)
>> #42: FILE: drivers/staging/media/av7110/av7110_av.c:778:
>> +               case PADDING_STREAM :
>>                                     ^
>>
>> ERROR: space prohibited before that ':' (ctx:WxE)
>> #43: FILE: drivers/staging/media/av7110/av7110_av.c:779:
>> +               case DSM_CC_STREAM  :
>>                                     ^
>> Can you fix that as well in a v2 of this patch?
> It seems that these spaces are deliberately added by the author to
> keep the case statements' colons aligned.
> Some other lines in the file where the same approach has been taken
> are [line 598,599,600,601] and [line 662, 663, 664, 665].
> Should we leave these spaces as it is?

Either just fix it here, or post a second patch that removes the spaces
throughout this driver. It's a very old driver, predating the much more
strict enforcement of coding style that we have today.

Regards,

	Hans

> 
> Thanks,
> Husni.
Ahamed Husni March 20, 2022, 4:42 p.m. UTC | #4 
Hello Hans,

> Either just fix it here, or post a second patch that removes the spaces
> throughout this driver. It's a very old driver, predating the much more
> strict enforcement of coding style that we have today.
I have sent the fix as two seperate patches. Please have a look.
https://lore.kernel.org/linux-staging/20220225155622.585621-1-ahamedhusni73@gmail.com/T/#t

Thanks,
Husni.
diff mbox series

Patch

diff --git a/drivers/staging/media/av7110/av7110_av.c b/drivers/staging/media/av7110/av7110_av.c
index 91f4866c7e59..1d42862e9669 100644
--- a/drivers/staging/media/av7110/av7110_av.c
+++ b/drivers/staging/media/av7110/av7110_av.c
@@ -770,22 +770,22 @@  static void p_to_t(u8 const *buf, long int length, u16 pid, u8 *counter,
 	if (length > 3 &&
 	     buf[0] == 0x00 && buf[1] == 0x00 && buf[2] == 0x01)
 		switch (buf[3]) {
-			case PROG_STREAM_MAP:
-			case PRIVATE_STREAM2:
-			case PROG_STREAM_DIR:
-			case ECM_STREAM     :
-			case EMM_STREAM     :
-			case PADDING_STREAM :
-			case DSM_CC_STREAM  :
-			case ISO13522_STREAM:
-			case PRIVATE_STREAM1:
-			case AUDIO_STREAM_S ... AUDIO_STREAM_E:
-			case VIDEO_STREAM_S ... VIDEO_STREAM_E:
-				pes_start = 1;
-				break;
+		case PROG_STREAM_MAP:
+		case PRIVATE_STREAM2:
+		case PROG_STREAM_DIR:
+		case ECM_STREAM     :
+		case EMM_STREAM     :
+		case PADDING_STREAM :
+		case DSM_CC_STREAM  :
+		case ISO13522_STREAM:
+		case PRIVATE_STREAM1:
+		case AUDIO_STREAM_S ... AUDIO_STREAM_E:
+		case VIDEO_STREAM_S ... VIDEO_STREAM_E:
+			pes_start = 1;
+			break;
 
-			default:
-				break;
+		default:
+			break;
 		}
 
 	while (c < length) {