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

Re: [XEN PATCH 01/13] xen/x86: fixed violations of MISRA C:2012 Rule 7.2



On Tue, 20 Jun 2023, Jan Beulich wrote:
> On 20.06.2023 12:34, Simone Ballarin wrote:
> > From: Gianluca Luparini <gianluca.luparini@xxxxxxxxxxx>
> > 
> > The xen sources contains violations of MISRA C:2012 Rule 7.2 whose headline 
> > states:
> > "A "u" or "U" suffix shall be applied to all integer constants that are 
> > represented in an unsigned type".
> > 
> > I propose to use "U" as a suffix to explicitly state when an integer 
> > constant is represented in an unsigned type.
> 
> The code adjustments here are certainly fine, but I'd like to ask that
> patch descriptions be written as such. "I propose ..." in particular
> may be okay in an upfront discussion, but for a patch you want to
> describe what the patch does, and why (the latter part you're dealing
> with already).
> 
> Furthermore I continue to have trouble with the wording "is represented
> in an unsigned type": As previously pointed out, what type a constant
> is going to be represented in depends on the ABI and eventual variables
> (specifically their types) that the value might then be assigned to, or
> expressions that the value might be used in. A possible future
> architecture with "int" wider than 32 bits would represent all the
> constants touched here in a signed type. I think what is meant instead
> (despite Misra's imo unhelpful wording) is that you add suffixes for
> constants which are meant to have unsigned values (no matter what type
> variable they would be stored in, or what expression they would appear
> in, and hence independent of their eventual representation).
> 
> Furthermore the U suffix (as an example) doesn't help at all when the
> value then is assigned to a variable of type long, and long is wider
> than int. The value would then _still_ be represented in a signed type.
> 
> Taken together, how about 'Use "U" as a suffix to explicitly state when
> an integer constant is intended to be an unsigned one'?
> 
> I expect both remarks will apply throughout the series, so I'm not
> going to repeat them for later patches.


Hi Jan, I agree with you. To further help Gianluca undestand better your
suggestion, I think the commit message wants to be:


    xen/x86/acpi/cpufreq: fixed violations of MISRA C:2012 Rule 7.2

    The xen sources contains violations of MISRA C:2012 Rule 7.2 whose
    headline states: "A "u" or "U" suffix shall be applied to all
    integer constants that are represented in an unsigned type".

    Use "U" as a suffix to explicitly state when an integer constant is
    intended to be an unsigned one

    For homogeneity, also add the "U" suffix in other cases that the
    tool didn't report as violations.


I also took the opportunity to make the title unique. Jan, if you are
happy with this wording it could be applied to all patches in this
series (with the titles being made unique).



 


Rackspace

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