Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NULL parameter checks in Zephyr APIs #21513

Closed
koffes opened this issue Dec 19, 2019 · 3 comments
Closed

NULL parameter checks in Zephyr APIs #21513

koffes opened this issue Dec 19, 2019 · 3 comments
Assignees
Labels
area: API Changes to public APIs Feature Request A request for a new feature

Comments

@koffes
Copy link
Collaborator

koffes commented Dec 19, 2019

Zephyr contains numerous APIs which require a pointer to be supplied as parameter which is not NULL.
Usually, the doc specifies this, but as we all know, developers make mistakes.
When a NULL pointer is supplied where it should not, may lead to an MPU_FAULT. When an MPU_FAULT occurs, this often requires more comprehensive knowledge of embedded systems, such as *.elf files, the addr2line tool and so forth to find the root cause.
For many of us, ease of development and customer satisfaction is key. For a user to get direct LOG output if NULL is supplied using e.g. __LINE__ and __FILE__ would greatly ease development.

In order not to bloat production code, I suggest making a NULL_PARAM_CHECK(var1, var2, ...) macro which in the inner workings prints __LINE__, __FILE__, flushes the LOG and ASSERTS. Alternatively, instead of asserting, invoking k_panic(). This param check macro can be enabled by default in DEBUG mode.

This issue has already been partially discussed here #19473.

@koffes koffes added the Feature Request A request for a new feature label Dec 19, 2019
@aescolar aescolar added the area: API Changes to public APIs label Dec 19, 2019
@ceolin
Copy link
Member

ceolin commented Mar 20, 2020

@andrewboie @koffes what exactly ? I mean, don't we have this features with __ASSERT ?
This macro even allows application decides which action takes.

@koffes
Copy link
Collaborator Author

koffes commented Mar 20, 2020

@ceolin We can absolutely use __ASSERT. Creating a separate macro was just an option. This issue was created in a broader sense, in that many places are missing checks for NULL pointers.

@carlescufi
Copy link
Member

Anything that supports userspace (syscalls) already verifies parameters when actually calling from userspace into kernel mode. Outside of userspace this is not done consistently, and could be done using the existing CHECKIF().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs Feature Request A request for a new feature
Projects
None yet
Development

No branches or pull requests

4 participants