Re: [PATCH v2 09/10] xsm: expand the function related macros in dummy.h

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 7/16/21 3:34 AM, Jan Beulich wrote:
> On 12.07.2021 22:32, Daniel P. Smith wrote:
>> With the elimination of switching how dummy.h gets included, the function
>> declaration macros are no longer necessary. This commit expands them out to 
>> the
>> only value for which they will ever be set. This results in function
>> declaration lengths changing and since some definitions did not even follow 
>> the
>> 80 column wrapping style, all function definitions were aligned with the
>> predominate style found in core hypervisor code.
> 
> I'm afraid this last half sentence is quite far from true:

I would disagree since I know I went through the frustration of trying
to find a discernible consistency in the files in common/ in the end I
settled on following common/memory.c since it seemed to have the most
uniform, it had only a couple of anomalies, as opposed to other files
where indentation was varied throughout.

>> @@ -82,43 +79,43 @@ static always_inline int xsm_default_action(
>>      }
>>  }
>>  
>> -static XSM_INLINE void dummy_security_domaininfo(struct domain *d,
>> +static inline void dummy_security_domaininfo(struct domain *d,
>>                                      struct xen_domctl_getdomaininfo *info)
> 
> Padding wasn't good here before, but you clearly do not change it to
> either of the forms we agreed on as being the goal for consistency:

Then that agreement should be document as CODING_STYLE only states:


Line Length
-----------

Lines should be less than 80 characters in length.  Long lines should
be split at sensible places and the trailing portions indented.


I found that in common/memory.c the predominate style was to align
parameters with the first parameter when wrapping, which is what I
followed. In this specific case when I wrapped the second parameter to
make the line less than 80 chars (an explicit rule in CODING_STYLE) and
attempted to align with the first paramter resulted in the line
exceeding 80 chars. Since the only hard rule is lines must be less than
80, I decreased the indent by enough characters for the line to be less
than 80 to be in line with CODING_STYLE since it only calls for sensible
splits that are indented.

> static inline void dummy_security_domaininfo(struct domain *d,
>                                              struct xen_domctl_getdomaininfo 
> *info)
> 
> or
> 
> static inline void dummy_security_domaininfo(
>     struct domain *d,
>     struct xen_domctl_getdomaininfo *info)
> 

I will align to the second, even though I find it annoying to switch
alignment styles, since the first would be in violation of CODING_STYLE
sine the second line would exceed 80 chars

>> -static XSM_INLINE int dummy_domain_create(XSM_DEFAULT_ARG struct domain *d, 
>> u32 ssidref)
>> +static inline int dummy_domain_create(struct domain *d, u32 ssidref)
> 
> When you have to touch lines anyway, may I suggest that you also take
> the opportunity and convert u<N> to uint<N>_t, to bring this file
> better in line with ./CODING_STYLE?

Sure.

v/r,
dps