Misra c round1 #91
Misra c round1 #91
Conversation
|
Placing this in review and will run CI on OSX ASAP |
mikaelarguedas
added
in review
|
MacOS CI passed. ready for review |
| @@ -219,7 +219,7 @@ rcutils_string_map_clear(rcutils_string_map_t * string_map) | |||
| return RCUTILS_RET_STRING_MAP_INVALID, rcutils_get_default_allocator()) | |||
| size_t i = 0; | |||
| for (; i < string_map->impl->capacity; ++i) { | |||
| if (string_map->impl->keys[i]) { | |||
| if (string_map->impl->keys[i] != NULL) { | |||
There was a problem hiding this comment.
Just a question, I notice it's consistent about NULL == something for equality and something != NULL for inequality. Is the order of these a rule in misra C?
There was a problem hiding this comment.
No that's not part of MISRA, but it's part of our design guildelines
Always place literals on the left hand side of comparison operators, e.g. 0 == ret instead of ret == 0
- rationale: ret == 0 too easily turns into ret = 0 by accident
There was a problem hiding this comment.
Is our rule for all comparison operators, or just ==? If the former then should this line be NULL != string_map->impl->keys[i])?
There was a problem hiding this comment.
Yeah I guess that would be better.
My reading was that it's a lot less likely to write = instead of != and more likely to write = instead of == and that's what we apply everywhere else in the codebase. But I'm not opposed to apply the rule to all comparison operators and not only ==
There was a problem hiding this comment.
!= becoming = seems a lot less likely. I'm not advocating changing anything, my question is answered by you saying it isn't part of Misra C
This addresses a subset of misra C violations in this package
/cc @dejanpan