Multi-slice bytes support #439
Conversation
fixed z_client_test checking string length; minor readability fixes;
|
|
||
| _z_arc_slice_t _z_arc_slice_get_subslice(const _z_arc_slice_t* s, size_t offset, size_t len) { | ||
| assert(offset + len <= s->len); | ||
| assert(s->slice.in != NULL || (len == 0 && offset == 0)); |
There was a problem hiding this comment.
Asserts will be removed on RELEASE compile, I'd say we should use them only in tests.
There was a problem hiding this comment.
This is internal function, thus I believe asserts here are justified, since they clearly represent the contract on input arguments (i.e. under no circumstances they are supposed to be violated, no matter what the user input would be to the public api).
There was a problem hiding this comment.
But if the check is important, assert is not ideal:
- They disappear when
NDEBUGis defined. - They don't make much sense on bare metal systems and as such might be unavailable.
- It violates Misra rule 21.8
The library functions abort, exit, getenv and system of shall not be used.
So if the error is somewhat recoverable, then the function should return an error which then propagate or is processed.
If it's truly unrecoverable, I guess we need to create a portable mechanism that doesn't violate the Misra rule.
There was a problem hiding this comment.
Yes, this error is supposed to be non-recoverable (but likely will be caught before in some higher level api and bubbled up through error propagation mechanism). But as I said it is internal function and thus asserts are supposed to help catching this violation during testing/development. Using asserts is really a standard practice in specifying contracts, I do not see why we should complicate it. They really contribute to a safer development process with little to no overhead.
There was a problem hiding this comment.
Ok I didn't understand the intent correctly. if it's used as in-place testing, then it's fine.
Maybe it could be worth to wrap them, in case we stumble upon an availability issue on some platforms but it's not in the scope of this PR.
| @@ -66,6 +66,11 @@ static inline void _z_noop_copy(void *dst, const void *src) { | |||
| (void)(src); | |||
| } | |||
|
|
|||
| static inline void _z_noop_move(void *dst, void *src) { | |||
| (void)(dst); | |||
| (void)(src); | |||
There was a problem hiding this comment.
Should use the _ZP_UNUSED macro for these.
|
Fixes (at least partially) #419 |
fixed public z_bytes api; added public z_bytes_api tests;
Multi-slice bytes support.
Support for pair / iterator serialization / deserialization;
Implementation of z_bytes_iterator, z_bytes_reade and z_bytes_writer; closes #445, closes #446
Implementation of z_bytes_empty(...), z_bytes_is_empty(...) and z_bytes_len(...); closes #444
closes #425