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

From: Ahamed Husni
Date: Tue Mar 08 2022 - 07:50:30 EST


Hello Hans,

On Mon, Mar 7, 2022 at 12:58 PM Hans Verkuil <hverkuil@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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.