outreachy.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] staging: most: fix line ending with '('
@ 2023-03-22 15:01 Khadija Kamran
  2023-03-22 15:09 ` Dan Carpenter
  2023-03-22 15:12 ` Randy Dunlap
  0 siblings, 2 replies; 7+ messages in thread
From: Khadija Kamran @ 2023-03-22 15:01 UTC (permalink / raw)
  To: outreachy
  Cc: Parthiban Veerasooran, Christian Gromm, Greg Kroah-Hartman,
	linux-staging, linux-kernel

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] staging: most: fix line ending with '('
  2023-03-22 15:01 [PATCH v2] staging: most: fix line ending with '(' Khadija Kamran
@ 2023-03-22 15:09 ` Dan Carpenter
  2023-03-22 18:06   ` Khadija Kamran
  2023-03-22 15:12 ` Randy Dunlap
  1 sibling, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2023-03-22 15:09 UTC (permalink / raw)
  To: Khadija Kamran
  Cc: outreachy, Parthiban Veerasooran, Christian Gromm,
	Greg Kroah-Hartman, linux-staging, linux-kernel

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] staging: most: fix line ending with '('
  2023-03-22 15:01 [PATCH v2] staging: most: fix line ending with '(' Khadija Kamran
  2023-03-22 15:09 ` Dan Carpenter
@ 2023-03-22 15:12 ` Randy Dunlap
  2023-03-23  4:14   ` Deepak R Varma
  1 sibling, 1 reply; 7+ messages in thread
From: Randy Dunlap @ 2023-03-22 15:12 UTC (permalink / raw)
  To: Khadija Kamran, outreachy
  Cc: Parthiban Veerasooran, Christian Gromm, Greg Kroah-Hartman,
	linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] staging: most: fix line ending with '('
  2023-03-22 15:09 ` Dan Carpenter
@ 2023-03-22 18:06   ` Khadija Kamran
  2023-03-23  4:48     ` Dan Carpenter
  0 siblings, 1 reply; 7+ messages in thread
From: Khadija Kamran @ 2023-03-22 18:06 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: outreachy, Parthiban Veerasooran, Christian Gromm,
	Greg Kroah-Hartman, linux-staging, linux-kernel

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
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] staging: most: fix line ending with '('
  2023-03-22 15:12 ` Randy Dunlap
@ 2023-03-23  4:14   ` Deepak R Varma
  0 siblings, 0 replies; 7+ messages in thread
From: Deepak R Varma @ 2023-03-23  4:14 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Khadija Kamran, outreachy, Parthiban Veerasooran,
	Christian Gromm, Greg Kroah-Hartman, linux-staging, linux-kernel,
	Andy Whitcroft, Joe Perches, Dwaipayan Ray, Lukas Bulwahn

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.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] staging: most: fix line ending with '('
  2023-03-22 18:06   ` Khadija Kamran
@ 2023-03-23  4:48     ` Dan Carpenter
  2023-03-23 12:06       ` Khadija Kamran
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2023-03-23  4:48 UTC (permalink / raw)
  To: Khadija Kamran
  Cc: outreachy, Parthiban Veerasooran, Christian Gromm,
	Greg Kroah-Hartman, linux-staging, linux-kernel

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] staging: most: fix line ending with '('
  2023-03-23  4:48     ` Dan Carpenter
@ 2023-03-23 12:06       ` Khadija Kamran
  0 siblings, 0 replies; 7+ messages in thread
From: Khadija Kamran @ 2023-03-23 12:06 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: outreachy, Parthiban Veerasooran, Christian Gromm,
	Greg Kroah-Hartman, linux-staging, linux-kernel

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
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-03-23 12:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-22 15:01 [PATCH v2] staging: most: fix line ending with '(' Khadija Kamran
2023-03-22 15:09 ` Dan Carpenter
2023-03-22 18:06   ` Khadija Kamran
2023-03-23  4:48     ` Dan Carpenter
2023-03-23 12:06       ` Khadija Kamran
2023-03-22 15:12 ` Randy Dunlap
2023-03-23  4:14   ` Deepak R Varma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).