CHECK: Alignment should match open parenthesis

CHECK: Alignment should match open parenthesis

[palik imre]

Sorry for the silly question, but I have some issues with this checkpatch.pl warning.

I mean Documentation/CodingStyle says:

Outside of comments, documentation and except in Kconfig, spaces are never
used for indentation, and the above example is deliberately broken.

But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?

Thanks,

Imre

CHECK: Alignment should match open parenthesis

[Nicholas Mc Guire]

On Fri, 13 Mar 2015, palik imre wrote:

> Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
> 
> I mean Documentation/CodingStyle says:
> 
> Outside of comments, documentation and except in Kconfig, spaces are never
> used for indentation, and the above example is deliberately broken.
> 
> But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
>
leading tabs *followed* by spaces to align parameters to a function are fine
but for basic block indentation you never use spaces. so

	func(param1,
	     param2);

would be allowed but not
 
	if (something) {
	    if(something else) {

HTH

thx!
hofrat

CHECK: Alignment should match open parenthesis

[palik imre]

On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> On Fri, 13 Mar 2015, palik imre wrote:
> 
> 
> > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
> >
> > I mean Documentation/CodingStyle says:
> >
> > Outside of comments, documentation and except in Kconfig, spaces are never
> > used for indentation, and the above example is deliberately broken.
> >
> > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
> 
> >
> leading tabs *followed* by spaces to align parameters to a function are fine

The emacs settings in Documentation/CodingStyle seem to contradict to you,
as it is set up to use c-lineup-arglist-tabs-only

> but for basic block indentation you never use spaces. so
> 
>     func(param1,
>         param2);
> 
> would be allowed but not
> 
>     if (something) {
>         if(something else) {
> 
> HTH
> 
> thx!
> hofrat

CHECK: Alignment should match open parenthesis

[Nicholas Mc Guire]

On Fri, 13 Mar 2015, palik imre wrote:

> On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > On Fri, 13 Mar 2015, palik imre wrote:
> > 
> > 
> > > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
> > >
> > > I mean Documentation/CodingStyle says:
> > >
> > > Outside of comments, documentation and except in Kconfig, spaces are never
> > > used for indentation, and the above example is deliberately broken.
> > >
> > > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
> > 
> > >
> > leading tabs *followed* by spaces to align parameters to a function are fine
> 
> The emacs settings in Documentation/CodingStyle seem to contradict to you,
> as it is set up to use c-lineup-arglist-tabs-only
>
The problem is that CodingStyle does not explicitly address
parameter alignment for functions that do not fit on a single line
but checkpatch.pl does

you can try it out - if you align to the opening braces with spaces
with preceding TABs it will not fuss and this is also common practice.

thx!
hofrat

CHECK: Alignment should match open parenthesis

[Nicholas Mc Guire]

On Fri, 13 Mar 2015, Nicholas Mc Guire wrote:

> On Fri, 13 Mar 2015, palik imre wrote:
> 
> > On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > > On Fri, 13 Mar 2015, palik imre wrote:
> > > 
> > > 
> > > > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
> > > >
> > > > I mean Documentation/CodingStyle says:
> > > >
> > > > Outside of comments, documentation and except in Kconfig, spaces are never
> > > > used for indentation, and the above example is deliberately broken.
> > > >
> > > > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
> > > 
> > > >
> > > leading tabs *followed* by spaces to align parameters to a function are fine
> > 
> > The emacs settings in Documentation/CodingStyle seem to contradict to you,
> > as it is set up to use c-lineup-arglist-tabs-only
> >
> The problem is that CodingStyle does not explicitly address
> parameter alignment for functions that do not fit on a single line
> but checkpatch.pl does
> 
> you can try it out - if you align to the opening braces with spaces
> with preceding TABs it will not fuss and this is also common practice.
>
here is a quick shot at summarizing this

 
If the parameter list to a functions would exceed the 80 char limit then break
it at the separators, and align to opening braces, e.g.:

			ret = fw_load_from_user_helper(fw, name, device,
						       opt_flags, timeout);

or:

	int =
	wait_for_completion_interruptible_timeout(data->completion,             
						  PMI_TIMEOUT);

Note that this is indented by tabs and then aligned with spaced to fit the 
opening braces. If you can not fit it even if you break the parameter list
at the commas then indent by tabs only but *significantly* to the left of
the opening braces, e.g.:

		int ret = wait_for_completion_interruptible_timeout( 
				&info->done, usecs_to_jiffies(TIMEOUT_US));


would be suprised if there is no writeup somewhere alredy
but I did not find this covered in Documentations anywhere.

thx!
hofrat

CHECK: Alignment should match open parenthesis

[Daniel Baluta]

On Fri, Mar 13, 2015 at 3:47 PM, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> On Fri, 13 Mar 2015, Nicholas Mc Guire wrote:
>
>> On Fri, 13 Mar 2015, palik imre wrote:
>>
>> > On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
>> > > On Fri, 13 Mar 2015, palik imre wrote:
>> > >
>> > >
>> > > > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
>> > > >
>> > > > I mean Documentation/CodingStyle says:
>> > > >
>> > > > Outside of comments, documentation and except in Kconfig, spaces are never
>> > > > used for indentation, and the above example is deliberately broken.
>> > > >
>> > > > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
>> > >
>> > > >
>> > > leading tabs *followed* by spaces to align parameters to a function are fine
>> >
>> > The emacs settings in Documentation/CodingStyle seem to contradict to you,
>> > as it is set up to use c-lineup-arglist-tabs-only
>> >
>> The problem is that CodingStyle does not explicitly address
>> parameter alignment for functions that do not fit on a single line
>> but checkpatch.pl does
>>
>> you can try it out - if you align to the opening braces with spaces
>> with preceding TABs it will not fuss and this is also common practice.
>>
> here is a quick shot at summarizing this
>
>
> If the parameter list to a functions would exceed the 80 char limit then break
> it at the separators, and align to opening braces, e.g.:
>
>                         ret = fw_load_from_user_helper(fw, name, device,
>                                                        opt_flags, timeout);
>
> or:
>
>         int =
>         wait_for_completion_interruptible_timeout(data->completion,
>                                                   PMI_TIMEOUT);
>
> Note that this is indented by tabs and then aligned with spaced to fit the
> opening braces. If you can not fit it even if you break the parameter list
> at the commas then indent by tabs only but *significantly* to the left of
> the opening braces, e.g.:
>
>                 int ret = wait_for_completion_interruptible_timeout(
>                                 &info->done, usecs_to_jiffies(TIMEOUT_US));
>
>
> would be suprised if there is no writeup somewhere alredy
> but I did not find this covered in Documentations anywhere.

I think it would be a coding idea to have this in CodingStyle doc :).

Daniel.

CHECK: Alignment should match open parenthesis

[Greg KH]

On Fri, Mar 13, 2015 at 04:35:30PM +0200, Daniel Baluta wrote:
> On Fri, Mar 13, 2015 at 3:47 PM, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > On Fri, 13 Mar 2015, Nicholas Mc Guire wrote:
> >
> >> On Fri, 13 Mar 2015, palik imre wrote:
> >>
> >> > On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> >> > > On Fri, 13 Mar 2015, palik imre wrote:
> >> > >
> >> > >
> >> > > > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
> >> > > >
> >> > > > I mean Documentation/CodingStyle says:
> >> > > >
> >> > > > Outside of comments, documentation and except in Kconfig, spaces are never
> >> > > > used for indentation, and the above example is deliberately broken.
> >> > > >
> >> > > > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
> >> > >
> >> > > >
> >> > > leading tabs *followed* by spaces to align parameters to a function are fine
> >> >
> >> > The emacs settings in Documentation/CodingStyle seem to contradict to you,
> >> > as it is set up to use c-lineup-arglist-tabs-only
> >> >
> >> The problem is that CodingStyle does not explicitly address
> >> parameter alignment for functions that do not fit on a single line
> >> but checkpatch.pl does
> >>
> >> you can try it out - if you align to the opening braces with spaces
> >> with preceding TABs it will not fuss and this is also common practice.
> >>
> > here is a quick shot at summarizing this
> >
> >
> > If the parameter list to a functions would exceed the 80 char limit then break
> > it at the separators, and align to opening braces, e.g.:
> >
> >                         ret = fw_load_from_user_helper(fw, name, device,
> >                                                        opt_flags, timeout);
> >
> > or:
> >
> >         int =
> >         wait_for_completion_interruptible_timeout(data->completion,
> >                                                   PMI_TIMEOUT);
> >
> > Note that this is indented by tabs and then aligned with spaced to fit the
> > opening braces. If you can not fit it even if you break the parameter list
> > at the commas then indent by tabs only but *significantly* to the left of
> > the opening braces, e.g.:
> >
> >                 int ret = wait_for_completion_interruptible_timeout(
> >                                 &info->done, usecs_to_jiffies(TIMEOUT_US));
> >
> >
> > would be suprised if there is no writeup somewhere alredy
> > but I did not find this covered in Documentations anywhere.
> 
> I think it would be a coding idea to have this in CodingStyle doc :).

Really?  Do you want to codify every tiny thing here?  Just use common
sense people, it's not that big of a deal...

greg k-h

CHECK: Alignment should match open parenthesis

[Nicholas Mc Guire]

On Fri, 13 Mar 2015, Daniel Baluta wrote:

> On Fri, Mar 13, 2015 at 3:47 PM, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > On Fri, 13 Mar 2015, Nicholas Mc Guire wrote:
> >
> >> On Fri, 13 Mar 2015, palik imre wrote:
> >>
> >> > On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> >> > > On Fri, 13 Mar 2015, palik imre wrote:
> >> > >
> >> > >
> >> > > > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
> >> > > >
> >> > > > I mean Documentation/CodingStyle says:
> >> > > >
> >> > > > Outside of comments, documentation and except in Kconfig, spaces are never
> >> > > > used for indentation, and the above example is deliberately broken.
> >> > > >
> >> > > > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
> >> > >
> >> > > >
> >> > > leading tabs *followed* by spaces to align parameters to a function are fine
> >> >
> >> > The emacs settings in Documentation/CodingStyle seem to contradict to you,
> >> > as it is set up to use c-lineup-arglist-tabs-only
> >> >
> >> The problem is that CodingStyle does not explicitly address
> >> parameter alignment for functions that do not fit on a single line
> >> but checkpatch.pl does
> >>
> >> you can try it out - if you align to the opening braces with spaces
> >> with preceding TABs it will not fuss and this is also common practice.
> >>
> > here is a quick shot at summarizing this
> >
> >
> > If the parameter list to a functions would exceed the 80 char limit then break
> > it at the separators, and align to opening braces, e.g.:
> >
> >                         ret = fw_load_from_user_helper(fw, name, device,
> >                                                        opt_flags, timeout);
> >
> > or:
> >
> >         int =
> >         wait_for_completion_interruptible_timeout(data->completion,
> >                                                   PMI_TIMEOUT);
> >
> > Note that this is indented by tabs and then aligned with spaced to fit the
> > opening braces. If you can not fit it even if you break the parameter list
> > at the commas then indent by tabs only but *significantly* to the left of
> > the opening braces, e.g.:
> >
> >                 int ret = wait_for_completion_interruptible_timeout(
> >                                 &info->done, usecs_to_jiffies(TIMEOUT_US));
> >
> >
> > would be suprised if there is no writeup somewhere alredy
> > but I did not find this covered in Documentations anywhere.
> 
> I think it would be a coding idea to have this in CodingStyle doc :).
>
At this level of detail it would be almost unmaintainable
and also you would end up with far too many rules to actually
keep them in mind. That is the job of tools like checkpatch.pl
they can have all the myriads of corner cases encoded without
creating a burden to the developers/maintainers.

thx!
hofrat 

CHECK: Alignment should match open parenthesis

[Daniel Baluta]

On Fri, Mar 13, 2015 at 5:33 PM, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> On Fri, 13 Mar 2015, Daniel Baluta wrote:
>
>> On Fri, Mar 13, 2015 at 3:47 PM, Nicholas Mc Guire <der.herr@hofr.at> wrote:
>> > On Fri, 13 Mar 2015, Nicholas Mc Guire wrote:
>> >
>> >> On Fri, 13 Mar 2015, palik imre wrote:
>> >>
>> >> > On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
>> >> > > On Fri, 13 Mar 2015, palik imre wrote:
>> >> > >
>> >> > >
>> >> > > > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
>> >> > > >
>> >> > > > I mean Documentation/CodingStyle says:
>> >> > > >
>> >> > > > Outside of comments, documentation and except in Kconfig, spaces are never
>> >> > > > used for indentation, and the above example is deliberately broken.
>> >> > > >
>> >> > > > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
>> >> > >
>> >> > > >
>> >> > > leading tabs *followed* by spaces to align parameters to a function are fine
>> >> >
>> >> > The emacs settings in Documentation/CodingStyle seem to contradict to you,
>> >> > as it is set up to use c-lineup-arglist-tabs-only
>> >> >
>> >> The problem is that CodingStyle does not explicitly address
>> >> parameter alignment for functions that do not fit on a single line
>> >> but checkpatch.pl does
>> >>
>> >> you can try it out - if you align to the opening braces with spaces
>> >> with preceding TABs it will not fuss and this is also common practice.
>> >>
>> > here is a quick shot at summarizing this
>> >
>> >
>> > If the parameter list to a functions would exceed the 80 char limit then break
>> > it at the separators, and align to opening braces, e.g.:
>> >
>> >                         ret = fw_load_from_user_helper(fw, name, device,
>> >                                                        opt_flags, timeout);
>> >
>> > or:
>> >
>> >         int =
>> >         wait_for_completion_interruptible_timeout(data->completion,
>> >                                                   PMI_TIMEOUT);
>> >
>> > Note that this is indented by tabs and then aligned with spaced to fit the
>> > opening braces. If you can not fit it even if you break the parameter list
>> > at the commas then indent by tabs only but *significantly* to the left of
>> > the opening braces, e.g.:
>> >
>> >                 int ret = wait_for_completion_interruptible_timeout(
>> >                                 &info->done, usecs_to_jiffies(TIMEOUT_US));
>> >
>> >
>> > would be suprised if there is no writeup somewhere alredy
>> > but I did not find this covered in Documentations anywhere.
>>
>> I think it would be a coding idea to have this in CodingStyle doc :).
>>
> At this level of detail it would be almost unmaintainable
> and also you would end up with far too many rules to actually
> keep them in mind. That is the job of tools like checkpatch.pl
> they can have all the myriads of corner cases encoded without
> creating a burden to the developers/maintainers.

I see. Thanks!

CHECK: Alignment should match open parenthesis

[Jeff Haran]

-----Original Message-----
From: kernelnewbies-bounces@kernelnewbies.org [mailto:kernelnewbies-bounces at kernelnewbies.org] On Behalf Of Nicholas Mc Guire
Sent: Friday, March 13, 2015 8:33 AM
To: Daniel Baluta
Cc: kernelnewbies at kernelnewbies.org; palik imre
Subject: Re: CHECK: Alignment should match open parenthesis

On Fri, 13 Mar 2015, Daniel Baluta wrote:

> On Fri, Mar 13, 2015 at 3:47 PM, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > On Fri, 13 Mar 2015, Nicholas Mc Guire wrote:
> >
> >> On Fri, 13 Mar 2015, palik imre wrote:
> >>
> >> > On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> >> > > On Fri, 13 Mar 2015, palik imre wrote:
> >> > >
> >> > >
> >> > > > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
> >> > > >
> >> > > > I mean Documentation/CodingStyle says:
> >> > > >
> >> > > > Outside of comments, documentation and except in Kconfig, 
> >> > > > spaces are never used for indentation, and the above example is deliberately broken.
> >> > > >
> >> > > > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
> >> > >
> >> > > >
> >> > > leading tabs *followed* by spaces to align parameters to a 
> >> > > function are fine
> >> >
> >> > The emacs settings in Documentation/CodingStyle seem to 
> >> > contradict to you, as it is set up to use 
> >> > c-lineup-arglist-tabs-only
> >> >
> >> The problem is that CodingStyle does not explicitly address 
> >> parameter alignment for functions that do not fit on a single line 
> >> but checkpatch.pl does
> >>
> >> you can try it out - if you align to the opening braces with spaces 
> >> with preceding TABs it will not fuss and this is also common practice.
> >>
> > here is a quick shot at summarizing this
> >
> >
> > If the parameter list to a functions would exceed the 80 char limit 
> > then break it at the separators, and align to opening braces, e.g.:
> >
> >                         ret = fw_load_from_user_helper(fw, name, device,
> >                                                        opt_flags, 
> > timeout);
> >
> > or:
> >
> >         int =
> >         wait_for_completion_interruptible_timeout(data->completion,
> >                                                   PMI_TIMEOUT);
> >
> > Note that this is indented by tabs and then aligned with spaced to 
> > fit the opening braces. If you can not fit it even if you break the 
> > parameter list at the commas then indent by tabs only but 
> > *significantly* to the left of the opening braces, e.g.:
> >
> >                 int ret = wait_for_completion_interruptible_timeout(
> >                                 &info->done, 
> > usecs_to_jiffies(TIMEOUT_US));
> >
> >
> > would be suprised if there is no writeup somewhere alredy but I did 
> > not find this covered in Documentations anywhere.
> 
> I think it would be a coding idea to have this in CodingStyle doc :).
>
At this level of detail it would be almost unmaintainable and also you would end up with far too many rules to actually keep them in mind. That is the job of tools like checkpatch.pl they can have all the myriads of corner cases encoded without creating a burden to the developers/maintainers.

thx!
hofrat 

I often see people writing posts to this list stating that they are want to get started and want to contribute something. I suppose this thread provides a suggestion for them. Take the latest version of checkpatch .pl and reverse engineer from it an update to the coding style document that documents in plain, human readable English what that script implements. Seems it bit back asswards to me for the requirements to derive from a script that implements them, but that seems to be where we are at. The very fact that this thread even exists, that the original question needed to be asked demonstrates that there's something missing in the style document.

Just my 2 cents worth,

Jeff Haran

CHECK: Alignment should match open parenthesis

[Nicholas Mc Guire]

On Fri, 13 Mar 2015, Jeff Haran wrote:

> -----Original Message-----
> From: kernelnewbies-bounces at kernelnewbies.org [mailto:kernelnewbies-bounces at kernelnewbies.org] On Behalf Of Nicholas Mc Guire
> Sent: Friday, March 13, 2015 8:33 AM
> To: Daniel Baluta
> Cc: kernelnewbies at kernelnewbies.org; palik imre
> Subject: Re: CHECK: Alignment should match open parenthesis
> 
> On Fri, 13 Mar 2015, Daniel Baluta wrote:
> 
> > On Fri, Mar 13, 2015 at 3:47 PM, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > > On Fri, 13 Mar 2015, Nicholas Mc Guire wrote:
> > >
> > >> On Fri, 13 Mar 2015, palik imre wrote:
> > >>
> > >> > On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > >> > > On Fri, 13 Mar 2015, palik imre wrote:
> > >> > >
> > >> > >
> > >> > > > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
> > >> > > >
> > >> > > > I mean Documentation/CodingStyle says:
> > >> > > >
> > >> > > > Outside of comments, documentation and except in Kconfig, 
> > >> > > > spaces are never used for indentation, and the above example is deliberately broken.
> > >> > > >
> > >> > > > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
> > >> > >
> > >> > > >
> > >> > > leading tabs *followed* by spaces to align parameters to a 
> > >> > > function are fine
> > >> >
> > >> > The emacs settings in Documentation/CodingStyle seem to 
> > >> > contradict to you, as it is set up to use 
> > >> > c-lineup-arglist-tabs-only
> > >> >
> > >> The problem is that CodingStyle does not explicitly address 
> > >> parameter alignment for functions that do not fit on a single line 
> > >> but checkpatch.pl does
> > >>
> > >> you can try it out - if you align to the opening braces with spaces 
> > >> with preceding TABs it will not fuss and this is also common practice.
> > >>
> > > here is a quick shot at summarizing this
> > >
> > >
> > > If the parameter list to a functions would exceed the 80 char limit 
> > > then break it at the separators, and align to opening braces, e.g.:
> > >
> > >                         ret = fw_load_from_user_helper(fw, name, device,
> > >                                                        opt_flags, 
> > > timeout);
> > >
> > > or:
> > >
> > >         int =
> > >         wait_for_completion_interruptible_timeout(data->completion,
> > >                                                   PMI_TIMEOUT);
> > >
> > > Note that this is indented by tabs and then aligned with spaced to 
> > > fit the opening braces. If you can not fit it even if you break the 
> > > parameter list at the commas then indent by tabs only but 
> > > *significantly* to the left of the opening braces, e.g.:
> > >
> > >                 int ret = wait_for_completion_interruptible_timeout(
> > >                                 &info->done, 
> > > usecs_to_jiffies(TIMEOUT_US));
> > >
> > >
> > > would be suprised if there is no writeup somewhere alredy but I did 
> > > not find this covered in Documentations anywhere.
> > 
> > I think it would be a coding idea to have this in CodingStyle doc :).
> >
> At this level of detail it would be almost unmaintainable and also you would end up with far too many rules to actually keep them in mind. That is the job of tools like checkpatch.pl they can have all the myriads of corner cases encoded without creating a burden to the developers/maintainers.
> 
> thx!
> hofrat 
> 
> I often see people writing posts to this list stating that they are want to get started and want to contribute something. I suppose this thread provides a suggestion for them. Take the latest version of checkpatch .pl and reverse engineer from it an update to the coding style document that documents in plain, human readable English what that script implements. Seems it bit back asswards to me for the requirements to derive from a script that implements them, but that seems to be where we are at. The very fact that this thread even exists, that the original question needed to be asked demonstrates that there's something missing in the style document.
>

If you look at things like MISRA-C - 200+ "Rules" to follow for safe code
it turns out that basically nobody is able to write MISRA compliant code
under consideration of all 200+ rules unless they have tools that tell them
what is wrong with enough information that it can be fixed. And at the same
time having those rules codified makes it hard (if not impossible) to
update/maintain these rules - lots of which are actually out of date or
overstated but now can't be "fixed" - It might help to have this up on a
web-page/wiki somewhere - but if you put this into Documentation/CodingStyle at
this granularity the rate of people reading and keeping it cache-hot while
coding would sharply decline.

thx!
hofrat

CHECK: Alignment should match open parenthesis

[Jeff Haran]

-----Original Message-----
From: Nicholas Mc Guire [mailto:der.herr at hofr.at] 
Sent: Friday, March 13, 2015 9:59 AM
To: Jeff Haran
Cc: Daniel Baluta; kernelnewbies at kernelnewbies.org; palik imre
Subject: Re: CHECK: Alignment should match open parenthesis

On Fri, 13 Mar 2015, Jeff Haran wrote:

> -----Original Message-----
> From: kernelnewbies-bounces at kernelnewbies.org 
> [mailto:kernelnewbies-bounces at kernelnewbies.org] On Behalf Of Nicholas 
> Mc Guire
> Sent: Friday, March 13, 2015 8:33 AM
> To: Daniel Baluta
> Cc: kernelnewbies at kernelnewbies.org; palik imre
> Subject: Re: CHECK: Alignment should match open parenthesis
> 
> On Fri, 13 Mar 2015, Daniel Baluta wrote:
> 
> > On Fri, Mar 13, 2015 at 3:47 PM, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > > On Fri, 13 Mar 2015, Nicholas Mc Guire wrote:
> > >
> > >> On Fri, 13 Mar 2015, palik imre wrote:
> > >>
> > >> > On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > >> > > On Fri, 13 Mar 2015, palik imre wrote:
> > >> > >
> > >> > >
> > >> > > > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
> > >> > > >
> > >> > > > I mean Documentation/CodingStyle says:
> > >> > > >
> > >> > > > Outside of comments, documentation and except in Kconfig, 
> > >> > > > spaces are never used for indentation, and the above example is deliberately broken.
> > >> > > >
> > >> > > > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
> > >> > >
> > >> > > >
> > >> > > leading tabs *followed* by spaces to align parameters to a 
> > >> > > function are fine
> > >> >
> > >> > The emacs settings in Documentation/CodingStyle seem to 
> > >> > contradict to you, as it is set up to use 
> > >> > c-lineup-arglist-tabs-only
> > >> >
> > >> The problem is that CodingStyle does not explicitly address 
> > >> parameter alignment for functions that do not fit on a single 
> > >> line but checkpatch.pl does
> > >>
> > >> you can try it out - if you align to the opening braces with 
> > >> spaces with preceding TABs it will not fuss and this is also common practice.
> > >>
> > > here is a quick shot at summarizing this
> > >
> > >
> > > If the parameter list to a functions would exceed the 80 char 
> > > limit then break it at the separators, and align to opening braces, e.g.:
> > >
> > >                         ret = fw_load_from_user_helper(fw, name, device,
> > >                                                        opt_flags, 
> > > timeout);
> > >
> > > or:
> > >
> > >         int =
> > >         wait_for_completion_interruptible_timeout(data->completion,
> > >                                                   PMI_TIMEOUT);
> > >
> > > Note that this is indented by tabs and then aligned with spaced to 
> > > fit the opening braces. If you can not fit it even if you break 
> > > the parameter list at the commas then indent by tabs only but
> > > *significantly* to the left of the opening braces, e.g.:
> > >
> > >                 int ret = wait_for_completion_interruptible_timeout(
> > >                                 &info->done, 
> > > usecs_to_jiffies(TIMEOUT_US));
> > >
> > >
> > > would be suprised if there is no writeup somewhere alredy but I 
> > > did not find this covered in Documentations anywhere.
> > 
> > I think it would be a coding idea to have this in CodingStyle doc :).
> >
> At this level of detail it would be almost unmaintainable and also you would end up with far too many rules to actually keep them in mind. That is the job of tools like checkpatch.pl they can have all the myriads of corner cases encoded without creating a burden to the developers/maintainers.
> 
> thx!
> hofrat
> 
> I often see people writing posts to this list stating that they are want to get started and want to contribute something. I suppose this thread provides a suggestion for them. Take the latest version of checkpatch .pl and reverse engineer from it an update to the coding style document that documents in plain, human readable English what that script implements. Seems it bit back asswards to me for the requirements to derive from a script that implements them, but that seems to be where we are at. The very fact that this thread even exists, that the original question needed to be asked demonstrates that there's something missing in the style document.
>

> If you look at things like MISRA-C - 200+ "Rules" to follow for safe code it turns out that basically nobody is able to write MISRA compliant code under consideration of all 200+ rules unless they have tools that tell them what is wrong with enough information that it can be fixed. And at the same time having those rules codified makes it hard (if not impossible) to update/maintain these rules - lots of which are actually out of date or overstated but now can't be "fixed" - It might help to have this up on a web-page/wiki somewhere - but if you put this into Documentation/CodingStyle at this granularity the rate of people reading and keeping it cache-hot while coding would sharply decline.

I'm not familiar with MISRA-C (it appears to be a non-open standard from the wiki I just read about it, that in of itself demonstrates a problem with it if the wiki is correct), but at least it appears that it attempts to document a standard that various tool implementers can follow if they choose to do so. If people coding to it don't want to bother to read it, they can choose to rely on the tools and if and when the tools generate different results, then there is a written standard that can be consulted to determine which tool is broken.

By what standard are changes to checkpatch.pl to be judged?

Jeff Haran

CHECK: Alignment should match open parenthesis

[Nicholas Mc Guire]

On Fri, 13 Mar 2015, Jeff Haran wrote:

> -----Original Message-----
> From: Nicholas Mc Guire [mailto:der.herr at hofr.at] 
> Sent: Friday, March 13, 2015 9:59 AM
> To: Jeff Haran
> Cc: Daniel Baluta; kernelnewbies at kernelnewbies.org; palik imre
> Subject: Re: CHECK: Alignment should match open parenthesis
> 
> On Fri, 13 Mar 2015, Jeff Haran wrote:
> 
> > -----Original Message-----
> > From: kernelnewbies-bounces at kernelnewbies.org 
> > [mailto:kernelnewbies-bounces at kernelnewbies.org] On Behalf Of Nicholas 
> > Mc Guire
> > Sent: Friday, March 13, 2015 8:33 AM
> > To: Daniel Baluta
> > Cc: kernelnewbies at kernelnewbies.org; palik imre
> > Subject: Re: CHECK: Alignment should match open parenthesis
> > 
> > On Fri, 13 Mar 2015, Daniel Baluta wrote:
> > 
> > > On Fri, Mar 13, 2015 at 3:47 PM, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > > > On Fri, 13 Mar 2015, Nicholas Mc Guire wrote:
> > > >
> > > >> On Fri, 13 Mar 2015, palik imre wrote:
> > > >>
> > > >> > On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > > >> > > On Fri, 13 Mar 2015, palik imre wrote:
> > > >> > >
> > > >> > >
> > > >> > > > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
> > > >> > > >
> > > >> > > > I mean Documentation/CodingStyle says:
> > > >> > > >
> > > >> > > > Outside of comments, documentation and except in Kconfig, 
> > > >> > > > spaces are never used for indentation, and the above example is deliberately broken.
> > > >> > > >
> > > >> > > > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
> > > >> > >
> > > >> > > >
> > > >> > > leading tabs *followed* by spaces to align parameters to a 
> > > >> > > function are fine
> > > >> >
> > > >> > The emacs settings in Documentation/CodingStyle seem to 
> > > >> > contradict to you, as it is set up to use 
> > > >> > c-lineup-arglist-tabs-only
> > > >> >
> > > >> The problem is that CodingStyle does not explicitly address 
> > > >> parameter alignment for functions that do not fit on a single 
> > > >> line but checkpatch.pl does
> > > >>
> > > >> you can try it out - if you align to the opening braces with 
> > > >> spaces with preceding TABs it will not fuss and this is also common practice.
> > > >>
> > > > here is a quick shot at summarizing this
> > > >
> > > >
> > > > If the parameter list to a functions would exceed the 80 char 
> > > > limit then break it at the separators, and align to opening braces, e.g.:
> > > >
> > > >                         ret = fw_load_from_user_helper(fw, name, device,
> > > >                                                        opt_flags, 
> > > > timeout);
> > > >
> > > > or:
> > > >
> > > >         int =
> > > >         wait_for_completion_interruptible_timeout(data->completion,
> > > >                                                   PMI_TIMEOUT);
> > > >
> > > > Note that this is indented by tabs and then aligned with spaced to 
> > > > fit the opening braces. If you can not fit it even if you break 
> > > > the parameter list at the commas then indent by tabs only but
> > > > *significantly* to the left of the opening braces, e.g.:
> > > >
> > > >                 int ret = wait_for_completion_interruptible_timeout(
> > > >                                 &info->done, 
> > > > usecs_to_jiffies(TIMEOUT_US));
> > > >
> > > >
> > > > would be suprised if there is no writeup somewhere alredy but I 
> > > > did not find this covered in Documentations anywhere.
> > > 
> > > I think it would be a coding idea to have this in CodingStyle doc :).
> > >
> > At this level of detail it would be almost unmaintainable and also you would end up with far too many rules to actually keep them in mind. That is the job of tools like checkpatch.pl they can have all the myriads of corner cases encoded without creating a burden to the developers/maintainers.
> > 
> > thx!
> > hofrat
> > 
> > I often see people writing posts to this list stating that they are want to get started and want to contribute something. I suppose this thread provides a suggestion for them. Take the latest version of checkpatch .pl and reverse engineer from it an update to the coding style document that documents in plain, human readable English what that script implements. Seems it bit back asswards to me for the requirements to derive from a script that implements them, but that seems to be where we are at. The very fact that this thread even exists, that the original question needed to be asked demonstrates that there's something missing in the style document.
> >
> 
> > If you look at things like MISRA-C - 200+ "Rules" to follow for safe code it turns out that basically nobody is able to write MISRA compliant code under consideration of all 200+ rules unless they have tools that tell them what is wrong with enough information that it can be fixed. And at the same time having those rules codified makes it hard (if not impossible) to update/maintain these rules - lots of which are actually out of date or overstated but now can't be "fixed" - It might help to have this up on a web-page/wiki somewhere - but if you put this into Documentation/CodingStyle at this granularity the rate of people reading and keeping it cache-hot while coding would sharply decline.
> 
> I'm not familiar with MISRA-C (it appears to be a non-open standard from the wiki I just read about it, that in of itself demonstrates a problem with it if the wiki is correct), but at least it appears that it attempts to document a standard that various tool implementers can follow if they choose to do so. If people coding to it don't want to bother to read it, they can choose to rely on the tools and if and when the tools generate different results, then there is a written standard that can be consulted to determine which tool is broken.
> 
> By what standard are changes to checkpatch.pl to be judged?
>
MISRA-C is a closed standard - probably not a good example of a
bad example :)

putting the rules into a specification file would make sense
but putting them into the CodingStyle directly I think would
be problematic - my take on the CodingStyle file is to 
get the general rules clarified in a way that it can be 
remembered relatively easy but not overload it with details.

most of the rules are outlined in the checkpatch.pl directly
in one/two line statements for the rule that started this
discusison.

# at the beginning of a line any tabs must come first and anything
# more than 8 must use tabs.

so maybe a solution to the missing complete set would be to
improve that in source description of each rule and add an
option to checkpatch.pl to list all of the rules. Not sure
if that makes sense but it would atleast keep things in
one location.

thx!
hofrat

CHECK: Alignment should match open parenthesis

[Jeff Haran]

-----Original Message-----
From: Nicholas Mc Guire [mailto:der.herr at hofr.at] 
Sent: Friday, March 13, 2015 10:31 AM
To: Jeff Haran
Cc: Daniel Baluta; kernelnewbies at kernelnewbies.org; palik imre
Subject: Re: CHECK: Alignment should match open parenthesis

On Fri, 13 Mar 2015, Jeff Haran wrote:

> -----Original Message-----
> From: Nicholas Mc Guire [mailto:der.herr at hofr.at]
> Sent: Friday, March 13, 2015 9:59 AM
> To: Jeff Haran
> Cc: Daniel Baluta; kernelnewbies at kernelnewbies.org; palik imre
> Subject: Re: CHECK: Alignment should match open parenthesis
> 
> On Fri, 13 Mar 2015, Jeff Haran wrote:
> 
> > -----Original Message-----
> > From: kernelnewbies-bounces at kernelnewbies.org
> > [mailto:kernelnewbies-bounces at kernelnewbies.org] On Behalf Of 
> > Nicholas Mc Guire
> > Sent: Friday, March 13, 2015 8:33 AM
> > To: Daniel Baluta
> > Cc: kernelnewbies at kernelnewbies.org; palik imre
> > Subject: Re: CHECK: Alignment should match open parenthesis
> > 
> > On Fri, 13 Mar 2015, Daniel Baluta wrote:
> > 
> > > On Fri, Mar 13, 2015 at 3:47 PM, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > > > On Fri, 13 Mar 2015, Nicholas Mc Guire wrote:
> > > >
> > > >> On Fri, 13 Mar 2015, palik imre wrote:
> > > >>
> > > >> > On Friday, 13 March 2015, 13:43, Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > > >> > > On Fri, 13 Mar 2015, palik imre wrote:
> > > >> > >
> > > >> > >
> > > >> > > > Sorry for the silly question, but I have some issues with this checkpatch.pl warning.
> > > >> > > >
> > > >> > > > I mean Documentation/CodingStyle says:
> > > >> > > >
> > > >> > > > Outside of comments, documentation and except in Kconfig, 
> > > >> > > > spaces are never used for indentation, and the above example is deliberately broken.
> > > >> > > >
> > > >> > > > But checkpatch.pl claims I should align to open parentheses.  These two things seem to be contradictory to me.  Could somebody clarify this?
> > > >> > >
> > > >> > > >
> > > >> > > leading tabs *followed* by spaces to align parameters to a 
> > > >> > > function are fine
> > > >> >
> > > >> > The emacs settings in Documentation/CodingStyle seem to 
> > > >> > contradict to you, as it is set up to use 
> > > >> > c-lineup-arglist-tabs-only
> > > >> >
> > > >> The problem is that CodingStyle does not explicitly address 
> > > >> parameter alignment for functions that do not fit on a single 
> > > >> line but checkpatch.pl does
> > > >>
> > > >> you can try it out - if you align to the opening braces with 
> > > >> spaces with preceding TABs it will not fuss and this is also common practice.
> > > >>
> > > > here is a quick shot at summarizing this
> > > >
> > > >
> > > > If the parameter list to a functions would exceed the 80 char 
> > > > limit then break it at the separators, and align to opening braces, e.g.:
> > > >
> > > >                         ret = fw_load_from_user_helper(fw, name, device,
> > > >                                                        
> > > > opt_flags, timeout);
> > > >
> > > > or:
> > > >
> > > >         int =
> > > >         wait_for_completion_interruptible_timeout(data->completion,
> > > >                                                   PMI_TIMEOUT);
> > > >
> > > > Note that this is indented by tabs and then aligned with spaced 
> > > > to fit the opening braces. If you can not fit it even if you 
> > > > break the parameter list at the commas then indent by tabs only 
> > > > but
> > > > *significantly* to the left of the opening braces, e.g.:
> > > >
> > > >                 int ret = wait_for_completion_interruptible_timeout(
> > > >                                 &info->done, 
> > > > usecs_to_jiffies(TIMEOUT_US));
> > > >
> > > >
> > > > would be suprised if there is no writeup somewhere alredy but I 
> > > > did not find this covered in Documentations anywhere.
> > > 
> > > I think it would be a coding idea to have this in CodingStyle doc :).
> > >
> > At this level of detail it would be almost unmaintainable and also you would end up with far too many rules to actually keep them in mind. That is the job of tools like checkpatch.pl they can have all the myriads of corner cases encoded without creating a burden to the developers/maintainers.
> > 
> > thx!
> > hofrat
> > 
> > I often see people writing posts to this list stating that they are want to get started and want to contribute something. I suppose this thread provides a suggestion for them. Take the latest version of checkpatch .pl and reverse engineer from it an update to the coding style document that documents in plain, human readable English what that script implements. Seems it bit back asswards to me for the requirements to derive from a script that implements them, but that seems to be where we are at. The very fact that this thread even exists, that the original question needed to be asked demonstrates that there's something missing in the style document.
> >
> 
> > If you look at things like MISRA-C - 200+ "Rules" to follow for safe code it turns out that basically nobody is able to write MISRA compliant code under consideration of all 200+ rules unless they have tools that tell them what is wrong with enough information that it can be fixed. And at the same time having those rules codified makes it hard (if not impossible) to update/maintain these rules - lots of which are actually out of date or overstated but now can't be "fixed" - It might help to have this up on a web-page/wiki somewhere - but if you put this into Documentation/CodingStyle at this granularity the rate of people reading and keeping it cache-hot while coding would sharply decline.
> 
> I'm not familiar with MISRA-C (it appears to be a non-open standard from the wiki I just read about it, that in of itself demonstrates a problem with it if the wiki is correct), but at least it appears that it attempts to document a standard that various tool implementers can follow if they choose to do so. If people coding to it don't want to bother to read it, they can choose to rely on the tools and if and when the tools generate different results, then there is a written standard that can be consulted to determine which tool is broken.
> 
> By what standard are changes to checkpatch.pl to be judged?
>
>MISRA-C is a closed standard - probably not a good example of a bad example :)
>
>putting the rules into a specification file would make sense but putting them into the CodingStyle directly I think would be problematic - my take on the CodingStyle file is to get the general rules clarified in a way that it can be remembered relatively easy but not overload it with details.
>
>most of the rules are outlined in the checkpatch.pl directly in one/two line statements for the rule that started this discusison.
>
># at the beginning of a line any tabs must come first and anything # more than 8 must use tabs.
>
>so maybe a solution to the missing complete set would be to improve that in source description of each rule and add an option to checkpatch.pl to list all of the rules. Not sure if that makes sense but it would atleast keep things in one location.

Still:

> By what standard are changes to checkpatch.pl to be judged?

Thanks,

Jeff Haran

CHECK: Alignment should match open parenthesis

[Nicholas Mc Guire]

<snip>
> > > 
> > > I often see people writing posts to this list stating that they are want to get started and want to contribute something. I suppose this thread provides a suggestion for them. Take the latest version of checkpatch .pl and reverse engineer from it an update to the coding style document that documents in plain, human readable English what that script implements. Seems it bit back asswards to me for the requirements to derive from a script that implements them, but that seems to be where we are at. The very fact that this thread even exists, that the original question needed to be asked demonstrates that there's something missing in the style document.
> > >
> > 
> > > If you look at things like MISRA-C - 200+ "Rules" to follow for safe code it turns out that basically nobody is able to write MISRA compliant code under consideration of all 200+ rules unless they have tools that tell them what is wrong with enough information that it can be fixed. And at the same time having those rules codified makes it hard (if not impossible) to update/maintain these rules - lots of which are actually out of date or overstated but now can't be "fixed" - It might help to have this up on a web-page/wiki somewhere - but if you put this into Documentation/CodingStyle at this granularity the rate of people reading and keeping it cache-hot while coding would sharply decline.
> > 
> > I'm not familiar with MISRA-C (it appears to be a non-open standard from the wiki I just read about it, that in of itself demonstrates a problem with it if the wiki is correct), but at least it appears that it attempts to document a standard that various tool implementers can follow if they choose to do so. If people coding to it don't want to bother to read it, they can choose to rely on the tools and if and when the tools generate different results, then there is a written standard that can be consulted to determine which tool is broken.
> > 
> > By what standard are changes to checkpatch.pl to be judged?
> >
> >MISRA-C is a closed standard - probably not a good example of a bad example :)
> >
> >putting the rules into a specification file would make sense but putting them into the CodingStyle directly I think would be problematic - my take on the CodingStyle file is to get the general rules clarified in a way that it can be remembered relatively easy but not overload it with details.
> >
> >most of the rules are outlined in the checkpatch.pl directly in one/two line statements for the rule that started this discusison.
> >
> ># at the beginning of a line any tabs must come first and anything # more than 8 must use tabs.
> >
> >so maybe a solution to the missing complete set would be to improve that in source description of each rule and add an option to checkpatch.pl to list all of the rules. Not sure if that makes sense but it would atleast keep things in one location.
> 
> Still:
> 
> > By what standard are changes to checkpatch.pl to be judged?
>

Documentation/CodingStyle is a sufficiently stable basis that the detailed
checker rules can then be changed by the normal consent achieved by posting
patches to checkpatch.pl to the appropriate maintainer+LKML.

An evolutionary approach is more robust that trying to put it
down in detail at one point - note that scripts/checkpatch.pl
has had about 440 patches applied in its life-time - 
  git log --pretty=format:"%s" scripts/checkpatch.pl | wc
so one could speculate that any CodingStyle that wanted to put
rules down in detail would end up being at least a few hundred 
rules long wich would be hard to keep synced in detail with
an evolving script aside from being hard to use for daily work.

Extracting the rule set so that it could be used by new 
developers would be sufficient to support new developers and
doing that from checkpatch.pl directly would allow to keep it
maintainable.

thx!
hofrat