-----Original Message----- From: Nicholas Mc Guire [mailto:[email protected]] Sent: Friday, March 13, 2015 10:31 AM To: Jeff Haran Cc: Daniel Baluta; [email protected]; 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:[email protected]] > Sent: Friday, March 13, 2015 9:59 AM > To: Jeff Haran > Cc: Daniel Baluta; [email protected]; palik imre > Subject: Re: CHECK: Alignment should match open parenthesis > > On Fri, 13 Mar 2015, Jeff Haran wrote: > > > -----Original Message----- > > From: [email protected] > > [mailto:[email protected]] On Behalf Of > > Nicholas Mc Guire > > Sent: Friday, March 13, 2015 8:33 AM > > To: Daniel Baluta > > Cc: [email protected]; 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 <[email protected]> > > > 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 > > > >> > <[email protected]> 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 _______________________________________________ Kernelnewbies mailing list [email protected] http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
