On 2024-02-29 17:25, Jan Beulich wrote:
On 29.02.2024 16:27, Nicola Vetrini wrote:
--- a/xen/include/xen/kconfig.h
+++ b/xen/include/xen/kconfig.h
@@ -25,7 +25,7 @@
#define __ARG_PLACEHOLDER_1 0,
#define config_enabled(cfg) _config_enabled(cfg)
#define _config_enabled(value)
__config_enabled(__ARG_PLACEHOLDER_##value)
-#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk
1, 0)
+#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk
(1), (0))
#define ___config_enabled(__ignored, val, ...) val
In addition to what Andrew said, would you mind clarifying what exactly
the
violation is here? I find it questionable that numeric literals need
parenthesizing; they don't normally need to, aynwhere.
This one's a little special. I couldn't parenthesize val further down,
because then it would break the build:
In file included from ././include/xen/config.h:14,
from <command-line>:
./include/xen/kconfig.h:29:52: error: expected identifier or ‘(’ before
‘)’ token
29 | #define ___config_enabled(__ignored, val, ...) (val)
| ^
./include/xen/kconfig.h:39:34: note: in expansion of macro
‘___config_enabled’
39 | #define _static_if(arg1_or_junk) ___config_enabled(arg1_or_junk
static,)
| ^~~~~~~~~~~~~~~~~
./include/xen/kconfig.h:38:26: note: in expansion of macro ‘_static_if’
38 | #define static_if(value) _static_if(__ARG_PLACEHOLDER_##value)
| ^~~~~~~~~~
./include/xen/kconfig.h:41:27: note: in expansion of macro ‘static_if’
41 | #define STATIC_IF(option) static_if(option)
| ^~~~~~~~~
common/page_alloc.c:260:1: note: in expansion of macro ‘STATIC_IF’
260 | STATIC_IF(CONFIG_NUMA) mfn_t first_valid_mfn =
INVALID_MFN_INITIALIZER;
| ^~~~~~~~~
make[2]: *** [Rules.mk:249: common/page_alloc.o] Error 1
--- a/xen/include/xen/list.h
+++ b/xen/include/xen/list.h
@@ -490,9 +490,9 @@ static inline void list_splice_init(struct
list_head *list,
* @member: the name of the list_struct within the struct.
*/
#define list_for_each_entry(pos, head, member)
\
- for (pos = list_entry((head)->next, typeof(*pos), member);
\
- &pos->member != (head);
\
- pos = list_entry(pos->member.next, typeof(*pos), member))
+ for (pos = list_entry((head)->next, typeof(*(pos)), member);
\
+ &(pos)->member != (head);
\
+ pos = list_entry((pos)->member.next, typeof(*(pos)),
member))
this ends up inconsistent, which I think isn't nice: Some uses of "pos"
are now parenthesized, while others aren't. Applies further down as
well.
Yeah, the inconsistency is due to the fact that a non-parenthesized
parameter as lhs is already deviated. To keep it consistent here I can
add parentheses, but then the deviation would be kind of pointless,
wouldn't it?
You may also want to take this as a strong suggestion to split
dissimilar
changes, so uncontroversial parts can go in.
Ok, that was an oversight.
@@ -977,4 +977,3 @@ static inline void hlist_add_after_rcu(struct
hlist_node *prev,
pos = pos->next)
#endif /* __XEN_LIST_H__ */
-
Unrelated change?
Oh, yes. Will drop it.
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -94,7 +94,7 @@ struct lock_profile_qhead {
int32_t idx; /* index for printout */
};
-#define _LOCK_PROFILE(lockname) { .name = #lockname, .lock =
&lockname, }
+#define _LOCK_PROFILE(lockname) { .name = #lockname, .lock =
&(lockname), }
This also may be viewed as falling in the same category, but is less
problematic because the other use is stringification, when in principle
some kind of expression would be passed in (albeit in practice I don't
expect anyone would do that).
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
