[ https://issues.apache.org/jira/browse/MESOS-7143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15873919#comment-15873919 ]
James Peach commented on MESOS-7143: ------------------------------------ Similarly the {{ABORT}} macro is optimistic. It lets you pass multiple arguments but {{_Abort}} only takes 1 message: {code} #define ABORT(...) _Abort(_ABORT_PREFIX, __VA_ARGS__) {code} For _Abort itself, consider annotating the arguments to be non-null and removing the checks. It's not cool to abort without an actionable message. > ABORT checks its preconditions incorrectly and incompletely > ----------------------------------------------------------- > > Key: MESOS-7143 > URL: https://issues.apache.org/jira/browse/MESOS-7143 > Project: Mesos > Issue Type: Bug > Components: stout > Affects Versions: 0.23.0 > Reporter: Benjamin Bannier > Priority: Minor > Labels: coverity, tech-debt > > Currently, stout's {{ABORT}} (which is mapped to {{_Abort}}) checks it > precondition incompletely and incorrectly. > Its current control flow is roughly > {code} > void _Abort(const char* prefix, const char* message) > { > size_t prefix_len = strlen(prefix); > size_t message_len = strlen(message); > > // Async-safe write. > while(::write(2, prefix, prefix_len) == -1 && errno == EINTR); > while(message != nullptr && > ::write(2, message, message_len) == -1 && errno == EINTR); > } > {code} > We here check the precondition {{message != nullptr}} after we already have > called {{strlen(message)}}; calling {{strlen}} on a {{nullptr}} already > triggers undefined behavior. > Similarly, we never guard against a {{prefix}} which is {{nullptr}}, but > unconditionally call {{strlen}} on it. > It seems it should be possible to assert that neither {{prefix}} nor > {{message}} are {{nullptr}} before any use. > This was diagnosed by coverity as CID-1400833, and has been present in all > releases since 0.23.0. -- This message was sent by Atlassian JIRA (v6.3.15#6346)