Re: [PATCH RFC net-next v2 2/3] vsock/test: Introduce get_transports()
From: Stefano Garzarella <sgarzare@redhat.com>
Date: 2025-06-11 14:21:00
Also in:
lkml, virtualization
On Fri, Jun 06, 2025 at 09:51:29AM +0200, Michal Luczaj wrote:
On 6/5/25 12:46, Stefano Garzarella wrote:quoted
On Wed, Jun 04, 2025 at 09:10:19PM +0200, Michal Luczaj wrote:quoted
On 6/4/25 11:07, Stefano Garzarella wrote:quoted
On Wed, May 28, 2025 at 10:44:42PM +0200, Michal Luczaj wrote:quoted
+static int __get_transports(void) +{ + /* Order must match transports defined in util.h. + * man nm: "d" The symbol is in the initialized data section. + */ + const char * const syms[] = { + "d loopback_transport", + "d virtio_transport", + "d vhost_transport", + "d vmci_transport", + "d hvs_transport", + };I would move this array (or a macro that define it), near the transport defined in util.h, so they are near and we can easily update/review changes. BTW what about adding static asserts to check we are aligned?Something like #define KNOWN_TRANSPORTS \What about KNOWN_TRANSPORTS(_) ?Ah, yeah.quoted
quoted
_(LOOPBACK, "loopback") \ _(VIRTIO, "virtio") \ _(VHOST, "vhost") \ _(VMCI, "vmci") \ _(HYPERV, "hvs") enum transport { TRANSPORT_COUNTER_BASE = __COUNTER__ + 1, #define _(name, symbol) \ TRANSPORT_##name = _BITUL(__COUNTER__ - TRANSPORT_COUNTER_BASE), KNOWN_TRANSPORTS TRANSPORT_NUM = __COUNTER__ - TRANSPORT_COUNTER_BASE, #undef _ }; static char * const transport_ksyms[] = { #define _(name, symbol) "d " symbol "_transport", KNOWN_TRANSPORTS #undef _ }; static_assert(ARRAY_SIZE(transport_ksyms) == TRANSPORT_NUM); ?Yep, this is even better, thanks :-)Although checkpatch complains: ERROR: Macros with complex values should be enclosed in parentheses #105: FILE: tools/testing/vsock/util.h:11: +#define KNOWN_TRANSPORTS(_) \ + _(LOOPBACK, "loopback") \ + _(VIRTIO, "virtio") \ + _(VHOST, "vhost") \ + _(VMCI, "vmci") \ + _(HYPERV, "hvs") BUT SEE: do {} while (0) advice is over-stated in a few situations: The more obvious case is macros, like MODULE_PARM_DESC, invoked at file-scope, where C disallows code (it must be in functions). See $exceptions if you have one to add by name. More troublesome is declarative macros used at top of new scope, like DECLARE_PER_CPU. These might just compile with a do-while-0 wrapper, but would be incorrect. Most of these are handled by detecting struct,union,etc declaration primitives in $exceptions. Theres also macros called inside an if (block), which "return" an expression. These cannot do-while, and need a ({}) wrapper. Enjoy this qualification while we work to improve our heuristics. ERROR: Macros with complex values should be enclosed in parentheses #114: FILE: tools/testing/vsock/util.h:20: + #define _(name, symbol) \ + TRANSPORT_##name = BIT(__COUNTER__ - TRANSPORT_COUNTER_BASE), WARNING: Argument 'symbol' is not used in function-like macro #114: FILE: tools/testing/vsock/util.h:20: + #define _(name, symbol) \ + TRANSPORT_##name = BIT(__COUNTER__ - TRANSPORT_COUNTER_BASE), WARNING: Argument 'name' is not used in function-like macro #122: FILE: tools/testing/vsock/util.h:28: + #define _(name, symbol) "d " symbol "_transport", Is it ok to ignore this? FWIW, I see the same ERRORs due to similarly used preprocessor directives in fs/bcachefs/alloc_background_format.h, and the same WARNINGs about unused macro arguments in arch/x86/include/asm/asm.h (e.g. __ASM_SEL).
It's just test, so I think it's fine to ignore, but please exaplain it in the commit description with also references to other ERRORs/WARNINGs like you did here. Let's see what net maintainers think. Thanks, Stefano