Re: [PATCH v4 04/11] xsm: apply coding style

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.09.2021 16:09, Daniel P. Smith wrote:
> On 9/7/21 9:50 AM, Jan Beulich wrote:
>> On 07.09.2021 15:41, Daniel P. Smith wrote:
>>> On 9/6/21 2:17 PM, Andrew Cooper wrote:
>>>> On 03/09/2021 20:06, Daniel P. Smith wrote:
>>>>> --- a/xen/include/xsm/dummy.h
>>>>> +++ b/xen/include/xsm/dummy.h
>>>>> @@ -69,8 +69,9 @@ void __xsm_action_mismatch_detected(void);
>>>>>    
>>>>>    #endif /* CONFIG_XSM */
>>>>>    
>>>>> -static always_inline int xsm_default_action(
>>>>> -    xsm_default_t action, struct domain *src, struct domain *target)
>>>>> +static always_inline int xsm_default_action(xsm_default_t action,
>>>>> +                                            struct domain *src,
>>>>> +                                            struct domain *target)
>>>>
>>>> The old code is correct.  We have plenty of examples of this in Xen, and
>>>> I have been adding new ones when appropriate.
>>>>
>>>> It avoids squashing everything on the RHS and ballooning the line count
>>>> to compensate.  (This isn't a particularly bad example, but we've had
>>>> worse cases in the past).
>>>
>>> Based on the past discussions I understood either is acceptable and find
>>> this version much easier to visually parse myself. With that said, if
>>> the "next line single indent" really is the preferred style by the
>>> maintainers/community, then I can convert all of these over.
>>
>> I guess neither is the "preferred" style; as Andrew says, both are
>> acceptable and both are in active use. I guess the rule of thumb is:
>> The longer what's left of the function name, the more you should
>> consider the style that you change away from.
>>
>> Anyway, in the end I guess the request for style adjustments was
>> mainly to purge bad style, not to convert one acceptable form to
>> another. Converting the entire file to the same style is of course
>> fine (for producing a consistent result), but then - as per above -
>> here it would more likely be the one that in this case was already
>> there.
> 
> Understood, I will respin with all the function defs to align with the 
> "next line single indent" style, though it would be helpful for 
> clarification on this style exactly. Do you always wrap all args if one 
> extends past 80 col or is there a rule for when some should remain on 
> the first line (function def line)?

I don't think that aspect has been discussed. I would say

void sufficiently_long_attribute test(unsigned int x, unsigned int y,
                                      unsigned int z, void *p);

is as acceptable as

void sufficiently_long_attribute test(unsigned int x,
                                      unsigned int y,
                                      unsigned int z,
                                      void *p);

with a slight preference to the former.

Jan