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

["Daniel P. Smith" <dpsmith@apertussolutions.com>]

On 9/7/21 10:27 AM, Jan Beulich wrote:
> 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
> 

Apologies, I was referring to this style which I am understanding is a
little more preferred

void short_function_name(
    struct really_long__struct_name *x,
    struct really_long__struct_name *y, unsigned int z, void *p);

vs

void short_function_name(struct really_long__struct_name *x,
    struct really_long__struct_name *y, unsigned int z, void *p);


NB: I don't recall it off the top of my head, but there is one function
def in here that is similar to this situation

v/r,
dps