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

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

Posted by Husni Faiz 4 years, 3 months ago

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) {
-- 
2.25.1

Re: [PATCH] media: av7110: av7110_av: Fix Switch and Case Same Indent Style Error

Posted by Hans Verkuil 4 years, 3 months ago

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

Re: [PATCH] media: av7110: av7110_av: Fix Switch and Case Same Indent Style Error

Posted by Ahamed Husni 4 years, 3 months ago

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.

Re: [PATCH] media: av7110: av7110_av: Fix Switch and Case Same Indent Style Error

Posted by Hans Verkuil 4 years, 3 months ago

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.

Re: [PATCH] media: av7110: av7110_av: Fix Switch and Case Same Indent Style Error

Posted by Ahamed Husni 4 years, 3 months ago

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.

[PATCH v2 1/2] media: av7110: fix switch indentation

Posted by Husni Faiz 4 years, 3 months ago

This patch fixes "switch and case should be at the same indent"
checkpatch error.

Signed-off-by: Husni Faiz <ahamedhusni73@gmail.com>
---
V1->V2: Add a clear subject line to the patch.

 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) {
-- 
2.25.1

Re: [PATCH v2 1/2] media: av7110: fix switch indentation

Posted by Ahamed Husni 4 years, 2 months ago

Ping :)

[PATCH v2 2/2] media: av7110: fix prohibited spaces in switch statement

Posted by Husni Faiz 4 years, 3 months ago

This patch fixes "space prohibited before that ':'" checkpatch error
in the switch statements.

Suggested-by: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Husni Faiz <ahamedhusni73@gmail.com>
---
V1->V2: Prohibited space fix after patch review.

 drivers/staging/media/av7110/av7110_av.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/av7110/av7110_av.c b/drivers/staging/media/av7110/av7110_av.c
index 1d42862e9669..ab7cf496b454 100644
--- a/drivers/staging/media/av7110/av7110_av.c
+++ b/drivers/staging/media/av7110/av7110_av.c
@@ -595,10 +595,10 @@ static int find_pes_header(u8 const *buf, long int length, int *frags)
 			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 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:
@@ -659,10 +659,10 @@ void av7110_p2t_write(u8 const *buf, long int length, u16 pid, struct av7110_p2t
 			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 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:
@@ -773,10 +773,10 @@ static void p_to_t(u8 const *buf, long int length, u16 pid, u8 *counter,
 		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 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:
-- 
2.25.1