-
Notifications
You must be signed in to change notification settings - Fork 23
storage: bm_storage: use errno instead of nrf_error #440
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
base: main
Are you sure you want to change the base?
storage: bm_storage: use errno instead of nrf_error #440
Conversation
|
You can find the documentation preview for this PR here. |
4e7be0e to
bcee21f
Compare
bcee21f to
594ff03
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could mention in the commit that
NRF_ERROR_INVALID_ADDR and NRF_ERROR_NULL are merged to -EFAULT, and
NRF_ERROR_INVALID_STATE and NRF_ERROR_FORBIDDEN are merged to -EPERM.
Also, is this something we want to do?
594ff03 to
dc0ef45
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this file? Looks like it was generated by nrfutil? It does probably not belong here and will be overwritten on next SoftDevice export.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, it should not be here. @nordicjm Should this be added to .gitignore or can it be rendered in the build folder or so instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
dc0ef45 to
7783e7f
Compare
|
|
||
| * Updated to use errno instead of nrf_errors. | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra newline
include/bm/storage/bm_storage.h
Outdated
| * 0 on success. | ||
| * A negative errno otherwise. | ||
| */ | ||
| uint32_t result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be int
| * @retval 0 on success. | ||
| * @retval -EFAULT If @p storage is @c NULL. | ||
| * @retval -EBUSY If the implementation-specific resource is busy. | ||
| * @retval -EIO If an implementation-specific internal error occurred. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use EFAULT for generic failures. IO is more for read/write and such.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use EFAULT for NULL checks.
| * @retval NRF_ERROR_BUSY If the implementation-specific resource is busy. | ||
| * @retval NRF_ERROR_INTERNAL If an implementation-specific internal error occurred. | ||
| * @retval 0 on success. | ||
| * @retval -EFAULT If @p storage is @c NULL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EINVAL? I prefer to use EINVAL here, but I know that we don't really have a standard for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the standard in the repo.
Fix compliance issue. Signed-off-by: Eivind Jølsgard <[email protected]>
Use errnos for BM storage as it is not a BLE library or BLE subsystem. Correct return values for BM ZMS. Signed-off-by: Eivind Jølsgard <[email protected]>
ring_buf_get() returns the size retrieved and will never be negative. Signed-off-by: Eivind Jølsgard <[email protected]>
7783e7f to
cd7a854
Compare
Use errnos for BM storage as it is not a BLE library or BLE subsystem.