Event based Bottom Half Processing failing to post an event

[steverpalmer]

Description

In my ISR routine I call bhp_event_isr_cb to defer the interrupt event handling to a high priority thread, which I understand to be the intent of this feature. However, the callback never occurs. I think I have debugged this back to an uninitialized clist pointer in a contained event_t structure.

In sys/bhp/event.c the function bhp_event_init correctly initializes the bhp_t structure (with a call to bhp_set_cb) and the event_queue_t pointer (simple assignment), but only partially initializes the event_t structure with an assignment to the .handler field. Specifically, the .list_node.next field remains unchanged by this function.

A subsequent call to bhp_event_isr_cb then calls event_post(bhp_event->evq, &bhp_event->ev) to push the event onto the appropriate queue, but event_post will only add the event to the queue (clist_rpush) if the .list_node.next field is NULL. I understand this is to allow event_post to be called multiple times (hopefully onto the same queue).

I suggest that the function bhp_event_init should initialize the whole event_t structure including the whole list_node field.

Steps to reproduce the issue

Consider:

#include <assert.h>

#include "event.h"
#include "bhp/event.h"

void dummy(void *) {}

int main(void) {
  event_queue_t queue = EVENT_QUEUE_INIT;
  bhp_event_t event;

  /*
   * `event` is likely to contain unitialized garbage,
   * but let's force the issue to best demonstrate the fault ...
   */
  event.ev.list_node.next = (void *)1;

  bhp_event_init(&event, &queue, dummy, NULL);

  // later, inside an Interrupt Service Routine ...

  bhp_event_isr_cb(&event);

  /*
   * If every thing has worked,
   * the event should be in the queue ...
   */
  assert (event_is_queued(&queue, &event.ev));
}

I've built the code with DEVELHELP=1 for the microbit-v2, and the program runs and asserts.

Expected results

No output (other than "main(): This is RIOT! ...").

Actual results

main(): This is RIOT! (Version: 2024.07-devel-174-g59956-2024-04)
137
FAILED ASSERTION.

Versions

RIOT version 2024-04

[steverpalmer]

Searching through the rest of the codebase, everywhere the bhp_event_t structure is used (i.e. gnrc/netif/init_devs.auto_init_* and in pkg/lwip/include/lwip/netif/compat.h) it is part of a static structure and so will be initialized to 0 at the start. Therefore, this "problem" does not get exposed elsewhere in the codebase.

[steverpalmer]

The comparable function event_callback_init starts by using memset(event_callback, 0, sizeof(*event_callback)) to clear out the structure.