Typecheck objects passed to K_POLL_EVENT_INITIALIZER

[arbrauns]

Too many times have I accidentally passed a struct k_sem ** or similar. Thankfully modern C can help us here.

[andyross]

Some notes. Seems clever.

[andyross]

Seems weird to abuse this "unused" field (which is a placeholder for extra bits that would be expected to go away if we found a use for them). Can't you rework the expression to take and evaluate to the _event_obj, e.g. ".obj = VALIDATE_OBJ(_event_obj)" or whatever?

[arbrauns]

I tried, haven't been able to come up with any reasonable way to "get rid" of the expression resulting from the _Static_assert. I'm all ears to hear from anyone with more cursed C knowledge than me though.

[andyross]

GCC statement expressions[1] are the normal tool for that. ({ STATEMENT_A; STATEMENT_B; }) executes A and then B, but then evaluates syntactically as the result of B.

[1] An extension we do allow, because it's so pervasively supported and incredibly useful. See https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html

[arbrauns]

Those aren't allowed outside of function scopes, which would be a weird restriction to impose.

[andyross]

As mentioned elsewhere, it's probably even more useful for this to take a parameter to which it will evaluate if it doesn't blow up.

[npitre]

You are missing K_POLL_TYPE_FIFO_DATA_AVAILABLE and have
K_POLL_TYPE_DATA_AVAILABLE twice.

I think this can be made much simpler while fully leveraging the compiler's
type checking ability, and also be C++ compatible. Consider this:

diff --git a/include/zephyr/kernel.h b/include/zephyr/kernel.h
index 32c554c159d..f4fb7765933 100644
--- a/include/zephyr/kernel.h
+++ b/include/zephyr/kernel.h
@@ -5686,14 +5686,14 @@ struct k_poll_event {
 
 	/** per-type data */
 	union {
-		void *obj;
-		struct k_poll_signal *signal;
-		struct k_sem *sem;
-		struct k_fifo *fifo;
-		struct k_queue *queue;
-		struct k_msgq *msgq;
+		void *obj, *typed_K_POLL_TYPE_IGNORE;
+		struct k_poll_signal *signal, *typed_K_POLL_TYPE_SIGNAL;
+		struct k_sem *sem, *typed_K_POLL_TYPE_SEM_AVAILABLE;
+		struct k_fifo *fifo, *typed_K_POLL_TYPE_FIFO_DATA_AVAILABLE;
+		struct k_queue *queue, *typed_K_POLL_TYPE_DATA_AVAILABLE;
+		struct k_msgq *msgq, *typed_K_POLL_TYPE_MSGQ_DATA_AVAILABLE;
 #ifdef CONFIG_PIPES
-		struct k_pipe *pipe;
+		struct k_pipe *pipe, *typed_K_POLL_TYPE_PIPE_DATA_AVAILABLE;
 #endif
 	};
 };
@@ -5706,7 +5706,7 @@ struct k_poll_event {
 	.mode = _event_mode, \
 	.unused = 0, \
 	{ \
-		.obj = _event_obj, \
+		.typed_##_event_type = _event_obj, \
 	}, \
 	}
 
@@ -5719,7 +5719,7 @@ struct k_poll_event {
 	.mode = _event_mode, \
 	.unused = 0, \
 	{ \
-		.obj = _event_obj, \
+		.typed_##_event_type = _event_obj, \
 	}, \
 	}
 

This even trips an existing test that does this:

        struct k_poll_event bad_events2[] = {
                K_POLL_EVENT_INITIALIZER(0xFU,
                                         K_POLL_MODE_NOTIFY_ONLY,
                                         &no_wait_sem),
        };

And I think that's a good thing. Initializers shouldn't be used with random
literals. The test could simply be modified to work around the initializer
in this particular case.

[cfriedt]

Sort of leaning toward Nico's solution and allowing the compiler to better do its job.

[arbrauns]

You are missing K_POLL_TYPE_FIFO_DATA_AVAILABLE and have
K_POLL_TYPE_DATA_AVAILABLE twice.

Those are the same thing.

Anyway, I've adopted your solution, it is nicer indeed.

[andyross]

Very clean