Hi Andrew,
Andrew Cooper writes:
> 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_babc...@epam.com>
>> ---
>> 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.
Excuse me, what it 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.
I totally agree with you. Probably we should either add a generic clause
like "This coding style rules may be violated if they produce weird
results".
Or we can add clarification to this particular rule: "Do not break
parameter definition to multiple lines. If parameters are too long,
decrease indentation, but try to line them up. e.g.:
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);
"
What do you think?
Problem is that document will blow up quickly if we'll try to cover all
corner cases. So personally I stick to "generic rules + common sense"
approach.
--
Volodymyr Babchuk at EPAM
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
