On 07/08/2023 10:31, Jan Beulich wrote:
On 04.08.2023 17:27, Nicola Vetrini wrote:
The macros defined 'xen/include/public/arch-x86/xen-mca.h' have
needless
underscore prefixes for parameter names and variable names that cause
shadowing with e.g. the variable 'i' in function 'mce_action'.
Therefore, the renaming aims to resolve present shadowing issues and
lessen the probability of future ones.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetr...@bugseng.com>
I'm okay with the code adjustments here, but I'm afraid I don't follow
the description: How is shadowing of "i" connected to the use of
leading underscores in macro parameter names? I think you need to
separate the two aspects in the wording.
The shadowing remark is tied to the second part of the sentence, but
I'll try
to make that clearer in v2.
--- a/xen/include/public/arch-x86/xen-mca.h
+++ b/xen/include/public/arch-x86/xen-mca.h
@@ -280,39 +280,39 @@ DEFINE_XEN_GUEST_HANDLE(xen_mc_logical_cpu_t);
/* Prototype:
* uint32_t x86_mcinfo_nentries(struct mc_info *mi);
*/
-#define x86_mcinfo_nentries(_mi) \
- (_mi)->mi_nentries
+#define x86_mcinfo_nentries(mi) \
+ (mi)->mi_nentries
Isn't there another rule demanding parenthization of the whole
construct? If so, adding the then-missing parentheses right here would
be quite desirable. (Personally I'm happy about them not being there on
suffix expressions, as kind of an exception to the general rule.)
If you're referring to Rule 20.7 then it does not require the whole
expression to be enclosed in
parentheses, as it's concerned with macro parameters (their full
expansion must parenthesize arguments at
some point, to prevent wrong evaluations due to operator precedence).
- if (_mi == NULL) break; \
- _mic = x86_mcinfo_first(_mi); \
- for (i = 0; i < x86_mcinfo_nentries(_mi); i++) { \
- if (_mic->type == (_type)) { \
- found = 1; \
+ found_ = 0; \
+ (ret) = NULL; \
+ if (mi == NULL) break; \
The lack of parentheses here definitely wants dealing with right away.
Jan
Good catch
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)