[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 16:27:03 +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=Q1oeAetJ19oaQfdWFwMctncqRNBSqpmyqIffsfr/z08=; b=O7+qhpaaGHOKIpSuxev6l4M+etsTAwpO+rZwaHXkEVgd3V4CEBPjYYuyZu7g8LZdkyN37Sa8GxDhXlTX1bMPkaxQAmGOlB4yvYr9Ul5dxPHfhP6WFPiCNuBlKRqay0b4HTxehQFzLmsSkGatJpGnj29rVquFRyXhCNsHSP9oIoFC3xh3kDNHb1hkbkzDBlCvsgvGI2saRJflbEBaukpgcl2G87zhsxsCJgxNc7mKyJnWJDR98LDURoVB4XZ/Z9mQup1NLAhEi3zKjJpeOVbgcAD8rZCB0Wu/zcCVexZe924WdEYXYQouuftiBOMxRWnJzStd3PxL0uHMM12b29Yvwg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UD29ztSBhiSO4Zn2b98F5Wm0ibcuYUAJdcXChpqY2WKA8DFeBkdJ6TQIS86QvLSXTEF0eBt3vuvm25LEgxVu8S7f0OrSIvOK8Ztqn/cNvVKSWSXbcAPI3lQEYekm77rUhuD9iE5609dCDHhtPNPBILWQBaejvJGPR5ZIaWfHFkJNOKIXkhKuqEDqe3jyhWLL6Mxa969BVhQPBf95vfedu/uOra2r5Y8UaRLYw6drTQMxArLARVsVHeV/uil+mF8GrN7H4n8pzGPwWGZ0bfpj6sjublabtZGgx7+8Ue8fhkfXGDt/cFszQqkGEKVNy6dEH+sOb+BUu79CNYQG8skaRQ==
  • 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 14:27:15 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.




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