On 20.06.2023 22:44, Stefano Stabellini wrote: > On Tue, 20 Jun 2023, Jan Beulich wrote: >> On 20.06.2023 12:34, Simone Ballarin wrote: >>> From: Gianluca Luparini <gianluca.lupar...@bugseng.com> >>> >>> 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).
Almost. In the case here perhaps: "x86/cpufreq: fix violations of MISRA C:2012 Rule 7.2". IOW I think subject prefixes shouldn't get too long, and past tense shouldn't be used unless describing an event in the past. As a minor further remark, the nested use of double quotes isn't very nice. When what is to be quoted contains double quotes, I would typically use single quotes around the construct. Jan
