Hello,
The fds.c fds_register() has suspicious usage of atomic variables.
ret_code_t fds_register(fds_cb_t cb)
{
ret_code_t ret;
if (m_users == FDS_MAX_USERS)
{
ret = FDS_ERR_USER_LIMIT_REACHED;
}
else
{
m_cb_table[m_users] = cb;
(void) nrf_atomic_u32_add(&m_users, 1);
ret = NRF_SUCCESS;
}
return ret;
}
It is first checked if there is space in the m_cb_table, then m_cb_table is modified and only then atomic add operation is completed.
It seems to me that the intention here was to do some sort of atomic compare-and-swap when checking for the users, for example
if (FDS_MAX_USERS <= nrf_atomic_u32_add(&m_users, 1))
But even that solution does not protect against e.g. interrupt trying to register another user to fds and incrementing m_users before m_cb_table is modified. I'm just nitpicking here in case m_users is atomic for some very good reason, so far I've never seen an issue which could be tracked down to concurrency problems here. On the other hand this function is the only place where atomic m_users is used, elsewhere the callback table is read until first NULL is found or max_users is reached.
