Code review comment for lp:~marcustomlinson/unity-scopes-api/debug_dbus_messages

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

> 93 + if (scope_started)
> ...
> 113 + else if (scope_started == false)
>
> A simple "else" would do.
>
> 28 + void publish_state_change(bool scope_started);
>
> Maybe use an enum Started, Stopped here? This would make the code at the call
> site much easier to read:
>
> publish_state_change(Started)
>
> instead of
>
> publish_state_change(true);

done

>
> if (std::system(started_message.c_str()) != 0)
>
> This looks like a crash or a hang waiting to happen.
>
> system() is not thread-safe. If we ever end up here concurrently (which is
> possible, I believe), we'll have a problem. I think it's necessary to
> serialise all calls to system.

« Back to merge proposal