[jira] [Commented] (MESOS-7143) ABORT checks its preconditions incorrectly and incompletely

["James Peach (JIRA)" <ji...@apache.org>]

    [ 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)