zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.72k stars 6.55k forks source link

Lack of a context parameter for callbacks leads to increased memory management burden for application developers... #51960

Open jimvert opened 1 year ago

jimvert commented 1 year ago
    As was mentioned by the first respondent, the presented method is a workaround.  As such, it doesn't address the core issue. 

In general, the lack of a "context" parameter is part of a broader API design issue which has to do with parameter lifetime. The documentation states that bt_gatt_write_params, for example, must remain valid for callback invocation. This means that both the bt_gatt_write_params structure and the "real" proposed C++ object context require separate memory management on behalf of the application developer.

One could effectively argue that few, if any elements of bt_gatt_write_params are interesting for callback execution, especially if one carries a separate "context" parameter into that callback. As such, something like bt_gatt_write, should copy the elements of bt_gatt_write_params necessary for the write operation. That would leave only the lifetime of the "context" parameter target to be managed by the application developer.

Again, this issue is more about API design decisions. I'm sure there is good rationale around those decisions, but they simply aren't clear from the documentation. Given the amount of experience garnered over the years that culminated into the Zephyr project, I would have expected closer scrutiny over those APIs...

Originally posted by @jimvert in https://github.com/zephyrproject-rtos/zephyr/issues/40560#issuecomment-1302041144

CodingVoid commented 1 year ago

In the documentation (doc/kernel/services/threads/workqueue.rst) it also recommends to use the CONTAINER_OF() Macro to use some kind of user configurable data for workqueues, which in my opinion comes straight from hell. There is definitely a need for some kind of "void *user_data".