Re: [Xen-devel] [PATCH] CODING_STYLE: clarify function argument indentation

[Lars Kurth <lars.kurth.xen@xxxxxxxxx>]

> On 31 Jul 2019, at 17:54, Viktor Mitin <viktor.mitin.19@xxxxxxxxx> wrote:
> 
> Hi All,
> 
> On Wed, Jul 31, 2019 at 7:45 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
> wrote:
>> 
>> On 31/07/2019 17:24, Volodymyr Babchuk wrote:
>>> There are coding style rules that are widely accepted by community,
>>> but newer were formalized in the document. Notable example is the
>>> question on how function arguments and parameters should be indented
>>> when they do not fit into one line.
>>> 
>>> This question was raised multiple times lately, mostly because of
>>> ongoing efforts to create Xen coding style formatting tool and because
>>> of new community members, who are not aware of such unwritten rules.
>>> 
>>> Actually, this rule is already implicitly defined in the document by
>>> defining emacs coding style: 'c-file-style: "BSD"'. In this mode emacs
>>> lines up function arguments under the first argument. Naturally, most
>>> of Xen code is written in this style.
>>> 
>>> So, lets state the obvious and fix this rule explicitly.
>>> 
>>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
>>> ---
>>> CODING_STYLE | 14 ++++++++++++++
>>> 1 file changed, 14 insertions(+)
>>> 
>>> diff --git a/CODING_STYLE b/CODING_STYLE
>>> index 6cc5b774cf..6479215a15 100644
>>> --- a/CODING_STYLE
>>> +++ b/CODING_STYLE
>>> @@ -53,6 +53,20 @@ Line Length
>>> Lines should be less than 80 characters in length.  Long lines should
>>> be split at sensible places and the trailing portions indented.
>>> 
>>> +For multiline function declaration and call each new line should be
>>> +aligned with the first the parameter or argument. e.g.:
>>> +
>>> +void my_function_with_long_name(struct lengthy_struct_name *struct1,
>>> +                                struct lengthy_struct_name *struct2,
>>> +                                struct lengthy_struct_name *struct3);
>>> +
>>> +or
>>> +
>>> +function_with_so_many_params(wordy_parameter1, wordy_parameter2,
>>> +                             wordy_parameter3, wordy_parameter4);
>>> +
>>> +The same applies for macros.
>> 
>> For very wordy functions, or ones with silly quantities of parameters,
>> the following is also acceptable
>> 
>> void my_function_with_long_and_silly_name(
>>    struct lengthy_struct_name *struct1, unsigned int womble, unsigned
>> int whatsit,
>>    struct lengthy_struct_name *struct2, bool yes, bool no, bool maybe,
>>    bool file_not_found, struct lengthy_struct_name *struct3, struct
>> lengthy_struct_name *struct4);
>> 
>> which you will find in a few places throughout the code, because the
>> above doesn't waste enough vertical space to fit several functions in,
>> and push all the relevant details to the RHS.
>> 
>> Per the above rules, the result would be this:
>> 
>> void my_function_with_long_and_silly_name(struct lengthy_struct_name
>> *struct1,
>>                                          unsigned int womble,
>>                                          unsigned int whatsit,
>>                                          struct lengthy_struct_name
>> *struct2,
>>                                          bool yes, bool no, bool maybe,
>>                                          bool file_not_found,
>>                                          struct lengthy_struct_name
>> *struct3,
>>                                          struct lengthy_struct_name
>> *struct4);
>> 
>> Of course, this is also a sign that maybe the function signature wants
>> changing anyway, but that may not be possible/sensible at the time.
>> 
>> As with everything, the coding style is a set of guidelines which are
>> applicable to 98% of cases, but there are cases where aren't
>> appropriate, and common sense is the only reasonable deciding factor.
> 
> It might be hard to automate 'common sense' cases. It seems it would
> be easier to find a solution on how to avoid such 'common sense'
> cases.
> 
> One more open point with this rule is how to format the next case where:
> len(return type string)+len(function name)+len(any of parameter) > 79
> 
> For example:
> +some_long_return_type  my_function_with_long_name(struct
> lengthy_struct_name *struct1,
> +                                struct lengthy_struct_name *struct2,
> +                                struct lengthy_struct_name *struct3);
> 
> Thanks

Ultimately we have to make some trade-offs as to what is more important:
a) automatic style checking - which means "common sense" can't be formalised 
and there will be boundary cases like the above
b) reclaiming code review bandwidth through automation or going for a labour 
intensive manual approach

I suggest we discuss in tomorrow's community call how to approach this.
I think the most important first step is to have a good view on the kind of 
boundary cases that we may face

Lars 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel