Code review comment for lp://staging/~unity-api-team/indicator-network/lp1411714-14.09

Revision history for this message
Jussi Pakkanen (jpakkane) wrote :

There are several calls like this:

std::unique_lock<std::mutex> lock(m_ofonoModem->_lock);
auto simmgr = m_ofonoModem->simManager.get();
auto netreg = m_ofonoModem->networkRegistration.get();
lock.unlock();

That is, the caller needs to be aware of the locking procedures. This is a potential future problem waiting to happen because when new calls to this function are added, one or more of them will forget to do this.

Could we instead change the getter function to be something like this:

void get(std::shared_ptr<Foo> &o) {
   // Comment saying why we do this with a link to the web page
   std::unique_lock<...> ...
   o = internal_variable;
}

In this way the get function is guaranteed to be thread safe as it can not be called incorrectly.

review: Needs Fixing

« Back to merge proposal