[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 7 Sep 2021 17:01:26 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=/yJIGk4ydRvmXHfQr1S7Kfi9GZbDReLS7yzFgjiLH+0=; b=I9Zge/jXxu5qsMi//mDK8p844hiS8Ypw5zIwzoxdvDtSs9ZsWwD9SjG8fNQz8I7TSs3sHBs1RwDuenq2y0eAd5W6z4HMgvplMzby5UfirsYL2Ktv1p0bvd1LAWPiDwg8Q8TYKvB7WQACo5Ey2Swg6XBGUueicIQysTp3iAW1y6WuZuVfnsnIUq/BSKB38Ko09u/euEjbCg98wCaOo/HmozVpkKFjbh3IjHVAMppbfR8Pu29YZUzuwETPOovORpF98B/Kv15vxE84eTSBJZKlFgMzA5uHJTASl7WA3RvtC0xOsVqA2larNfkAxaTYzaUpY1rGEO+mtEubcU88tNqWHA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=IOA/6gS4pY6AnCYtILAANLrTVUfP0X+rxkVrL7ehjnxI5881dchcqvIsVqieEqCLr2xwUfd/X0e42p0u0E0E/YOl+frFa6wd5CpAWm05eS4wDEV5sxGyFG3YWp5xtehHFok8UkicEHsFYGxBzw30odhX9E4E+tHAwP7ljIy3DzFclwZ8sUsTSc5TcRTa4Isc2bCbNG5/1UufvrU56Jh+qpevb8zMa+wctRv+6AGUFYpXo+XUj5xJ1ocF8q4II6bWBs4MzKmWAnXjE8Itg6cpHUNqHOn3ubVtslrsw7oecB5Xhf3JBGlqREmCiMxo88jwc7Z21PIpG8zuWFoJ05vTKg==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Tue, 07 Sep 2021 15:01:39 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 07.09.2021 16:55, Daniel P. Smith wrote:
> 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.
> 
> 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);

This latter style is not supposed to be used.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.