-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Description
Summary
Zephyr's API Design guidelines has the following:
The final parameter should be a void *user_data pointer carrying context that allows a shared callback function to locate additional material necessary to process the callback.
Zephyr's Coding Guidelines has rule 70 that prohibits the removal of const
when casting (even to void *
).
This means that in any case where there are some const
data that you'd would like to pass onto a callback for any such APIs that use void *user_data
, there will be a coding guideline compliance issue.
For example, a common example of this is the scan result callback in Bluetooth that may look like this:
static void device_found(const bt_addr_le_t *addr, int8_t rssi, uint8_t type,
struct net_buf_simple *ad)
{
char dev[BT_ADDR_LE_STR_LEN];
bt_addr_le_to_str(addr, dev, sizeof(dev));
printk("[DEVICE]: %s, AD evt type %u, AD data len %u, RSSI %i\n",
dev, type, ad->len, rssi);
/* We're only interested in legacy connectable events or
* possible extended advertising that are connectable.
*/
if (type == BT_GAP_ADV_TYPE_ADV_IND ||
type == BT_GAP_ADV_TYPE_ADV_DIRECT_IND ||
type == BT_GAP_ADV_TYPE_EXT_ADV) {
bt_data_parse(ad, eir_found, (void *)addr);
}
}
where bt_data_parse
is
void bt_data_parse(struct net_buf_simple *ad,
bool (*func)(struct bt_data *data, void *user_data),
void *user_data);
Our coding guidelines will prohibit this.
Other cases could be drivers that does
static int uarte_async_init(const struct device *dev)
{
struct uarte_nrfx_data *data = dev->data;
NRF_UARTE_Type *uarte = get_uarte_instance(dev);
static const uint32_t rx_int_mask =
NRF_UARTE_INT_ENDRX_MASK |
NRF_UARTE_INT_RXSTARTED_MASK |
NRF_UARTE_INT_ERROR_MASK |
NRF_UARTE_INT_RXTO_MASK |
((IS_ENABLED(CONFIG_UART_NRFX_UARTE_ENHANCED_RX) &&
!IS_ENABLED(UARTE_HAS_FRAME_TIMEOUT)) ? NRF_UARTE_INT_RXDRDY_MASK : 0);
#if !defined(CONFIG_UART_NRFX_UARTE_ENHANCED_RX)
int ret = uarte_nrfx_rx_counting_init(dev);
if (ret != 0) {
return ret;
}
#endif
nrf_uarte_int_enable(uarte, rx_int_mask);
k_timer_init(&data->async->rx.timer, rx_timeout, NULL);
k_timer_user_data_set(&data->async->rx.timer, (void *)dev);
k_timer_init(&data->async->tx.timer, tx_timeout, NULL);
k_timer_user_data_set(&data->async->tx.timer, (void *)dev);
return 0;
}
where const
is also removed from dev
when passing to __syscall void k_timer_user_data_set(struct k_timer *timer, void *user_data);
At the time of writing this, there are 1.9k violations of this rule in Zephyr.
Describe the solution you'd like
I don't see any easy solution for this. The following could be options:
- Add an exception for the rule for
void *
- Remove the rule from the coding guidelines
- Remove the API Design guideline
- Instead of supplying the pointer directly, make a copy and supply that (where applicable)
- Instead of supplying the pointer directly, store the pointer in a global variable and access that in the callback (where applicable)
- Add duplication of APIs that use user_data that support supplying
const void *user_data
Alternatives
No response
Additional Context
See also my original comment on Discord that received no responses (at the time of writing this): https://discord.com/channels/720317445772017664/1375902831927890041