Thread (14 messages) 14 messages, 2 authors, 2025-06-11

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help