sys_slist_append question

[AndyM]

Hello

In NCS 2.6.2 (v3.5.99-ncs1-2), lorawan_register_downlink_callback() directly calls sys_slist_append(&dl_callbacks, &cb->node) with no duplicate check.

My concern is that sys_slist_t is intrusive and each sys_snode_t has only one next pointer: sys_slist_append() first sets node->next = NULL, then does list->tail->next = node; therefore, if the same node is already the tail, appending it again appears to result in node->next = node, creating a self-referencing list and potentially an infinite traversal.

For example, if the list is A -> NULL and A is appended again, the result seems to become A -> A -> ...; if the node is already somewhere inside a longer list, clearing its next before re-appending could also break the existing chain.

I also noticed other Zephyr users of sys_slist_append() explicitly check whether a node is already registered or remove it before re-appending, which seems to support the idea that duplicate insertion is not expected.

Is the intended behavior that a sys_snode_t should never be appended while it is already linked? I started digging into this while investigating a crash in the LorarWAN subsystem. Found that my code in some cases called lorawan_register_downlink_callback()  multiple times. That API  directly calls sys_slist_append(&dl_callbacks, &cb->node) without checking if the node is already on the list. So the theory is that the crash was caused by traversing a corrupt list. But then the question is,why wasn't this noticed before - sys_slist_append is called throughout the SDK code and I couldn't find anything in the forums regarding  this
Thank you
Andy

[AndyM]

Just for posterity, no action needed, in case someone else stumbles into this
Nordic AI answer

Your analysis is correct, and this is a well-known hazard with intrusive singly-linked lists in Zephyr.

The Core Issue with sys_slist_t and Duplicate Insertion

The sys_slist_t is an intrusive data structure where each sys_snode_t contains only a single next pointer. [Single-linked List] Because the node itself is the linkage metadata, appending the same node twice corrupts the list — the node's next pointer is overwritten, breaking whatever chain it was already part of.

Your scenario is accurate:

• List: A -> NULL, tail = A
• sys_slist_append() sets A->next = NULL (no-op here), then tail->next = A, i.e. A->next = A
• Result: A -> A -> ... (infinite loop / self-referencing cycle)

If the node is in the middle of a longer list (e.g. A -> B -> C -> NULL), re-appending B would set B->next = NULL first, severing C from the list entirely, then link B after the tail — corrupting the list in a different way.

This is consistent with what the NCS Academy documentation explicitly warns about for FIFOs (which also use linked lists internally):

"It's important to remember that you can NOT add the same data item twice in a FIFO. This is likely to break the linked list used by the FIFO internally, and results in undefined behavior." [FIFO / Data Passing]

The same principle applies directly to sys_slist_append().

Why lorawan_register_downlink_callback() Has No Guard

The knowledge sources don't contain the LoRaWAN subsystem source, so I can't speak definitively to the design intent there. However, the general Zephyr sys_slist API provides no built-in duplicate detection — that is by design, as the library is intentionally minimal and unsynchronized:

"These libraries are generally unsynchronized; access to them is not threadsafe by default. These are data structures, not synchronization primitives. The expectation is that any locking needed will be provided by the user." [Data Structures]

The API contract is that the caller is responsible for not inserting a node that is already linked. The sys_dlist variant offers sys_dnode_is_linked() to check this, but sys_slist has no equivalent — checking requires a linear scan.

Why It May Not Have Been Caught Before

Most call sites register a callback once at initialization and never call the registration function again, so the duplicate-insertion path is rarely exercised. Your code hitting it in certain cases is an edge case that the LoRaWAN subsystem apparently doesn't guard against.

Recommended Fix

Before calling lorawan_register_downlink_callback(), ensure the node is not already registered. Since sys_slist has no is_linked check, the safest approaches are:

1. Call registration only once (e.g. guard with a flag).
2. Use sys_slist_find_and_remove() before re-appending, to safely remove the node if it is already present before adding it again.
3. Check cb->node.next and whether cb is the tail — but this is fragile and not recommended.

Option 1 is the simplest and most robust. The crash you observed is consistent with list corruption from a duplicate append, and your root cause analysis appears sound.