[PATCH v2] staging: most: fix line ending with '('

[PATCH v2] staging: most: fix line ending with '('

[Khadija Kamran]

Splitting function header to multiple lines because of 80 characters per
line limit, results in ending the function call line with '('.
This leads to CHECK reported by checkpatch.pl

Move the function parameters right after the '(' in the function call
line. Align the rest of the parameters to the opening parenthesis.

Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
---

Changes in v2:
 - Changed the description

 drivers/staging/most/dim2/hal.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/dim2/hal.c b/drivers/staging/most/dim2/hal.c
index a5d40b5b138a..6abe3ab2b2cf 100644
--- a/drivers/staging/most/dim2/hal.c
+++ b/drivers/staging/most/dim2/hal.c
@@ -346,9 +346,8 @@ static void dim2_clear_ctram(void)
 		dim2_clear_ctr(ctr_addr);
 }

-static void dim2_configure_channel(
-	u8 ch_addr, u8 type, u8 is_tx, u16 dbr_address, u16 hw_buffer_size,
-	u16 packet_length)
+static void dim2_configure_channel(u8 ch_addr, u8 type, u8 is_tx, u16 dbr_address,
+				   u16 hw_buffer_size, u16 packet_length)
 {
 	dim2_configure_cdt(ch_addr, dbr_address, hw_buffer_size, packet_length);
 	dim2_configure_cat(MLB_CAT, ch_addr, type, is_tx ? 1 : 0);
--
2.34.1

Re: [PATCH v2] staging: most: fix line ending with '('

[Dan Carpenter]

On Wed, Mar 22, 2023 at 08:01:52PM +0500, Khadija Kamran wrote:
> Splitting function header to multiple lines because of 80 characters per
> line limit, results in ending the function call line with '('.
> This leads to CHECK reported by checkpatch.pl
> 
> Move the function parameters right after the '(' in the function call
> line. Align the rest of the parameters to the opening parenthesis.
> 
> Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> ---
> 
> Changes in v2:
>  - Changed the description
> 

No one replied to the v1 of this patch so it's not clear which of the
patches should be ignored.

Also you have a couple patches with the same subject and that's against
the rules.

regards,
dan carpenter

Re: [PATCH v2] staging: most: fix line ending with '('

[Khadija Kamran]

On Wed, Mar 22, 2023 at 06:09:50PM +0300, Dan Carpenter wrote:
> On Wed, Mar 22, 2023 at 08:01:52PM +0500, Khadija Kamran wrote:
> > Splitting function header to multiple lines because of 80 characters per
> > line limit, results in ending the function call line with '('.
> > This leads to CHECK reported by checkpatch.pl
> > 
> > Move the function parameters right after the '(' in the function call
> > line. Align the rest of the parameters to the opening parenthesis.
> > 
> > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > ---
> > 
> > Changes in v2:
> >  - Changed the description
> > 
> 
> No one replied to the v1 of this patch so it's not clear which of the
> patches should be ignored.
>

Hey Dan!

I created a patch revision without any feedback. My bad. I thought the
description in the previous patch was misleading, so I wanted to change
it.

> Also you have a couple patches with the same subject and that's against
> the rules.
>

I am sorry about that. Kindly guide me here. 
Both patches are for same driver i.e. staging/most, but for different
files. Would it be fine to add file names in the subject to
differentiate them?

Regards,
Khadija

> regards,
> dan carpenter
> 

Re: [PATCH v2] staging: most: fix line ending with '('

[Dan Carpenter]

On Wed, Mar 22, 2023 at 11:06:07PM +0500, Khadija Kamran wrote:
> On Wed, Mar 22, 2023 at 06:09:50PM +0300, Dan Carpenter wrote:
> > On Wed, Mar 22, 2023 at 08:01:52PM +0500, Khadija Kamran wrote:
> > > Splitting function header to multiple lines because of 80 characters per
> > > line limit, results in ending the function call line with '('.
> > > This leads to CHECK reported by checkpatch.pl
> > > 
> > > Move the function parameters right after the '(' in the function call
> > > line. Align the rest of the parameters to the opening parenthesis.
> > > 
> > > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > > ---
> > > 
> > > Changes in v2:
> > >  - Changed the description
> > > 
> > 
> > No one replied to the v1 of this patch so it's not clear which of the
> > patches should be ignored.
> >
> 
> Hey Dan!
> 
> I created a patch revision without any feedback. My bad. I thought the
> description in the previous patch was misleading, so I wanted to change
> it.

Yeah.  But you have to reply to the other email.  Otherwise how will we
know there was a response to it?

> 
> > Also you have a couple patches with the same subject and that's against
> > the rules.
> >
> 
> I am sorry about that. Kindly guide me here. 
> Both patches are for same driver i.e. staging/most, but for different
> files. Would it be fine to add file names in the subject to
> differentiate them?
> 

I trust you.  Figure out a way.  ;)

regards,
dan carpenter

Re: [PATCH v2] staging: most: fix line ending with '('

[Khadija Kamran]

On Thu, Mar 23, 2023 at 07:48:11AM +0300, Dan Carpenter wrote:
> On Wed, Mar 22, 2023 at 11:06:07PM +0500, Khadija Kamran wrote:
> > On Wed, Mar 22, 2023 at 06:09:50PM +0300, Dan Carpenter wrote:
> > > On Wed, Mar 22, 2023 at 08:01:52PM +0500, Khadija Kamran wrote:
> > > > Splitting function header to multiple lines because of 80 characters per
> > > > line limit, results in ending the function call line with '('.
> > > > This leads to CHECK reported by checkpatch.pl
> > > > 
> > > > Move the function parameters right after the '(' in the function call
> > > > line. Align the rest of the parameters to the opening parenthesis.
> > > > 
> > > > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > > > ---
> > > > 
> > > > Changes in v2:
> > > >  - Changed the description
> > > > 
> > > 
> > > No one replied to the v1 of this patch so it's not clear which of the
> > > patches should be ignored.
> > >
> > 
> > Hey Dan!
> > 
> > I created a patch revision without any feedback. My bad. I thought the
> > description in the previous patch was misleading, so I wanted to change
> > it.
> 
> Yeah.  But you have to reply to the other email.  Otherwise how will we
> know there was a response to it?
>

Hey Dan,

I will keep that in mind next time.
Sorry about this.

Regards,
Khadija

> > 
> > > Also you have a couple patches with the same subject and that's against
> > > the rules.
> > >
> > 
> > I am sorry about that. Kindly guide me here. 
> > Both patches are for same driver i.e. staging/most, but for different
> > files. Would it be fine to add file names in the subject to
> > differentiate them?
> > 
> 
> I trust you.  Figure out a way.  ;)
> 
> regards,
> dan carpenter
> 

Re: [PATCH v2] staging: most: fix line ending with '('

[Randy Dunlap]

Hi--

On 3/22/23 08:01, Khadija Kamran wrote:
> Splitting function header to multiple lines because of 80 characters per
> line limit, results in ending the function call line with '('.
> This leads to CHECK reported by checkpatch.pl

Note that checkpatch is not always correct. It's a simple advisor.

> Move the function parameters right after the '(' in the function call
> line. Align the rest of the parameters to the opening parenthesis.

coding-style.rst does indeed say:

  A very commonly used style
  is to align descendants to a function open parenthesis.

That may be poorly worded. What it is trying to say is to align
the rest of the parameters to the first parameter immediately after
the open parenthesis, like this example from kernel/fork.c:

	stack = __vmalloc_node_range(THREAD_SIZE, THREAD_ALIGN,
				     VMALLOC_START, VMALLOC_END,
				     THREADINFO_GFP & ~__GFP_ACCOUNT,
				     PAGE_KERNEL,
				     0, node, __builtin_return_address(0));


And FTR, I still haven't found anything in coding-style.rst that says that
lines should not end with an open parenthesis.  Did I overlook it?

Thanks.

> Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> ---
> 
> Changes in v2:
>  - Changed the description
> 
>  drivers/staging/most/dim2/hal.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/most/dim2/hal.c b/drivers/staging/most/dim2/hal.c
> index a5d40b5b138a..6abe3ab2b2cf 100644
> --- a/drivers/staging/most/dim2/hal.c
> +++ b/drivers/staging/most/dim2/hal.c
> @@ -346,9 +346,8 @@ static void dim2_clear_ctram(void)
>  		dim2_clear_ctr(ctr_addr);
>  }
> 
> -static void dim2_configure_channel(
> -	u8 ch_addr, u8 type, u8 is_tx, u16 dbr_address, u16 hw_buffer_size,
> -	u16 packet_length)
> +static void dim2_configure_channel(u8 ch_addr, u8 type, u8 is_tx, u16 dbr_address,
> +				   u16 hw_buffer_size, u16 packet_length)
>  {
>  	dim2_configure_cdt(ch_addr, dbr_address, hw_buffer_size, packet_length);
>  	dim2_configure_cat(MLB_CAT, ch_addr, type, is_tx ? 1 : 0);
> --
> 2.34.1
> 

-- 
~Randy

Re: [PATCH v2] staging: most: fix line ending with '('

[Deepak R Varma]

On Wed, Mar 22, 2023 at 08:12:23AM -0700, Randy Dunlap wrote:
> Hi--
> 
> And FTR, I still haven't found anything in coding-style.rst that says that
> lines should not end with an open parenthesis.  Did I overlook it?

Hi Randy,
You are correct. I did not find anything clear or obvious that calls out not
ending the lines with a '(' symbol. 
I think the other guideline from the coding style is to align the overrunning
argument list in the next lines such that they match the arg1 after '('. So, I
think that may be the reason why checkpatch is coded to catch such lines.

Regards,
Deepak.

> 
> Thanks.