Commit 66140094 authored by Guillaume Roguez's avatar Guillaume Roguez

fix bad usage of shared_ptr as function argument

Many function/method pass shared_ptr by copy and not by (const) reference.
As explained here [1], if the smast pointer is not supposed to be modified
and used as an output argument, it's a bad usage as we cause
an uneeded copy. As this copy causes a memory lock this lead to CPU
and memory cache lock bad usage.

This patch converts all these by-value into by-const-reference
on all shared_ptr arguments over the daemon sources.

[1] https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/

Change-Id: Ic1e68d93a6917f5e2865b6818b7c1210ff98b1a8
parent 490ed632
......@@ -331,8 +331,8 @@ Account::setActiveCodecs(const std::vector<unsigned>& list)
std::sort(std::begin(accountCodecInfoList_),
std::end (accountCodecInfoList_),
[](std::shared_ptr<AccountCodecInfo> a,
std::shared_ptr<AccountCodecInfo> b) {
[](const std::shared_ptr<AccountCodecInfo>& a,
const std::shared_ptr<AccountCodecInfo>& b) {
return a->order < b->order;
});
......
......@@ -464,7 +464,7 @@ Call::addSubCall(const std::shared_ptr<Call>& call)
}
void
Call::merge(std::shared_ptr<Call> scall)
Call::merge(const std::shared_ptr<Call>& scall)
{
RING_WARN("[call:%s] merge to -> [call:%s]", scall->getCallId().c_str(), getCallId().c_str());
auto& call = *scall;
......
......@@ -344,7 +344,7 @@ class Call : public Recordable, public std::enable_shared_from_this<Call> {
}
void addSubCall(const std::shared_ptr<Call>& call);
virtual void merge(std::shared_ptr<Call> scall);
virtual void merge(const std::shared_ptr<Call>& scall);
protected:
/**
......
......@@ -118,7 +118,7 @@ class CallbackWrapper : public CallbackWrapperBase {
// Create and initialize a wrapper from a generic CallbackWrapperBase
// shared pointer.
// Note: the given callback is copied into internal storage.
CallbackWrapper(std::shared_ptr<CallbackWrapperBase> p) noexcept {
CallbackWrapper(const std::shared_ptr<CallbackWrapperBase>& p) noexcept {
if (p)
cb_ = ((CallbackWrapper<TProto>*)p.get())->cb_;
}
......
......@@ -453,7 +453,7 @@ Manager::switchCall(const std::string& id)
}
void
Manager::switchCall(std::shared_ptr<Call> call)
Manager::switchCall(const std::shared_ptr<Call>& call)
{
switchCall(call->getCallId());
}
......@@ -1445,7 +1445,7 @@ Manager::scheduleTask(const std::function<void()>&& task, std::chrono::steady_cl
void
Manager::scheduleTask(std::shared_ptr<Runnable> task, std::chrono::steady_clock::time_point when)
Manager::scheduleTask(const std::shared_ptr<Runnable>& task, std::chrono::steady_clock::time_point when)
{
std::lock_guard<std::mutex> lock(scheduledTasksMutex_);
scheduledTasks_.emplace(when, task);
......
......@@ -809,7 +809,7 @@ class Manager {
void unsetCurrentCall();
void switchCall(const std::string& id);
void switchCall(std::shared_ptr<Call> call);
void switchCall(const std::shared_ptr<Call>& call);
/** Application wide tone controler */
ToneControl toneCtrl_;
......@@ -998,7 +998,7 @@ class Manager {
Runnable(const std::function<void()>&& t) : cb(std::move(t)) {}
};
std::shared_ptr<Runnable> scheduleTask(const std::function<void()>&& task, std::chrono::steady_clock::time_point when);
void scheduleTask(std::shared_ptr<Runnable> task, std::chrono::steady_clock::time_point when);
void scheduleTask(const std::shared_ptr<Runnable>& task, std::chrono::steady_clock::time_point when);
#ifdef RING_VIDEO
/**
......
......@@ -142,7 +142,7 @@ RingBufferPool::removeReadBindings(const std::string& call_id)
* Make given call ID a reader of given ring buffer
*/
void
RingBufferPool::addReaderToRingBuffer(std::shared_ptr<RingBuffer> rbuf,
RingBufferPool::addReaderToRingBuffer(const std::shared_ptr<RingBuffer>& rbuf,
const std::string& call_id)
{
if (call_id != DEFAULT_ID and rbuf->id == call_id)
......@@ -154,7 +154,7 @@ RingBufferPool::addReaderToRingBuffer(std::shared_ptr<RingBuffer> rbuf,
}
void
RingBufferPool::removeReaderFromRingBuffer(std::shared_ptr<RingBuffer> rbuf,
RingBufferPool::removeReaderFromRingBuffer(const std::shared_ptr<RingBuffer>& rbuf,
const std::string& call_id)
{
if (auto bindings = getReadBindings(call_id)) {
......
......@@ -135,10 +135,10 @@ class RingBufferPool {
void removeReadBindings(const std::string& call_id);
void addReaderToRingBuffer(std::shared_ptr<RingBuffer> rbuf,
void addReaderToRingBuffer(const std::shared_ptr<RingBuffer>& rbuf,
const std::string& call_id);
void removeReaderFromRingBuffer(std::shared_ptr<RingBuffer> rbuf,
void removeReaderFromRingBuffer(const std::shared_ptr<RingBuffer>& rbuf,
const std::string& call_id);
// A cache of created RingBuffers listed by IDs.
......
......@@ -314,7 +314,7 @@ SinkClient::SinkClient(const std::string& id, bool mixer)
void
SinkClient::update(Observable<std::shared_ptr<VideoFrame>>* /*obs*/,
std::shared_ptr<VideoFrame> frame_p)
const std::shared_ptr<VideoFrame>& frame_p)
{
auto& f = *frame_p;
......
......@@ -56,7 +56,7 @@ class SinkClient : public VideoFramePassiveReader
// as VideoFramePassiveReader
void update(Observable<std::shared_ptr<ring::VideoFrame>>*,
std::shared_ptr<ring::VideoFrame>) override;
const std::shared_ptr<ring::VideoFrame>&) override;
bool start() noexcept;
bool stop() noexcept;
......
......@@ -116,7 +116,7 @@ class Observer
{
public:
virtual ~Observer() {};
virtual void update(Observable<T>*, T) = 0;
virtual void update(Observable<T>*, const T&) = 0;
virtual void attached(Observable<T>*) {};
virtual void detached(Observable<T>*) {};
};
......
......@@ -101,7 +101,7 @@ VideoMixer::detached(Observable<std::shared_ptr<VideoFrame>>* ob)
void
VideoMixer::update(Observable<std::shared_ptr<VideoFrame>>* ob,
std::shared_ptr<VideoFrame> frame_p)
const std::shared_ptr<VideoFrame>& frame_p)
{
auto lock(rwMutex_.read());
......
......@@ -49,7 +49,7 @@ public:
int getPixelFormat() const override;
// as VideoFramePassiveReader
void update(Observable<std::shared_ptr<VideoFrame>>* ob, std::shared_ptr<VideoFrame> v) override;
void update(Observable<std::shared_ptr<VideoFrame>>* ob, const std::shared_ptr<VideoFrame>& v) override;
void attached(Observable<std::shared_ptr<VideoFrame>>* ob) override;
void detached(Observable<std::shared_ptr<VideoFrame>>* ob) override;
......
......@@ -75,7 +75,7 @@ VideoSender::encodeAndSendVideo(VideoFrame& input_frame)
void
VideoSender::update(Observable<std::shared_ptr<VideoFrame>>* /*obs*/,
std::shared_ptr<VideoFrame> frame_p)
const std::shared_ptr<VideoFrame>& frame_p)
{
encodeAndSendVideo(*frame_p);
}
......
......@@ -54,7 +54,7 @@ public:
// as VideoFramePassiveReader
void update(Observable<std::shared_ptr<VideoFrame>>* obs,
std::shared_ptr<VideoFrame> frame_p) override;
const std::shared_ptr<VideoFrame>& frame_p) override;
void setMuted(bool isMuted);
uint16_t getLastSeqValue();
......
......@@ -113,8 +113,8 @@ void NameDirectory::lookupAddress(const std::string& addr, LookupCallback cb)
RING_DBG("Address lookup for %s: %s", addr.c_str(), uri.to_string().c_str());
auto ret = restbed::Http::async(req, [this,cb,addr](const std::shared_ptr<restbed::Request>,
const std::shared_ptr<restbed::Response> reply) {
auto ret = restbed::Http::async(req, [this,cb,addr](const std::shared_ptr<restbed::Request>&,
const std::shared_ptr<restbed::Response>& reply) {
if (reply->get_status_code() == 200) {
size_t length = getContentLength(*reply);
if (length > MAX_RESPONSE_SIZE) {
......@@ -180,8 +180,8 @@ void NameDirectory::lookupName(const std::string& n, LookupCallback cb)
RING_DBG("Name lookup for %s: %s", name.c_str(), uri.to_string().c_str());
auto ret = restbed::Http::async(request, [this,cb,name](const std::shared_ptr<restbed::Request>,
const std::shared_ptr<restbed::Response> reply) {
auto ret = restbed::Http::async(request, [this,cb,name](const std::shared_ptr<restbed::Request>&,
const std::shared_ptr<restbed::Response>& reply) {
auto code = reply->get_status_code();
if (code != 200)
RING_DBG("Name lookup for %s: got reply code %d", name.c_str(), code);
......@@ -272,8 +272,8 @@ void NameDirectory::registerName(const std::string& addr, const std::string& n,
RING_WARN("registerName: sending request %s %s", addr.c_str(), name.c_str());
auto ret = restbed::Http::async(request,
[this,cb,addr,name](const std::shared_ptr<restbed::Request>,
const std::shared_ptr<restbed::Response> reply)
[this,cb,addr,name](const std::shared_ptr<restbed::Request>&,
const std::shared_ptr<restbed::Response>& reply)
{
auto code = reply->get_status_code();
RING_DBG("Got reply for registration of %s -> %s: code %d", name.c_str(), addr.c_str(), code);
......
......@@ -1172,7 +1172,7 @@ RingAccount::loadAccountFromDHT(const std::string& archive_password, const std::
std::tie(key, loc) = computeKeys(archive_password, archive_pin, previous);
RING_DBG("Trying to load account from DHT with %s at %s", archive_pin.c_str(), loc.toString().c_str());
if (auto this_ = w.lock()) {
this_->dht_.get(loc, [w,key,found,archive_password,archiveFound](std::shared_ptr<dht::Value> val) {
this_->dht_.get(loc, [w,key,found,archive_password,archiveFound](const std::shared_ptr<dht::Value>& val) {
std::vector<uint8_t> decrypted;
try {
decrypted = dht::crypto::aesDecrypt(val->data, key);
......@@ -1961,7 +1961,7 @@ RingAccount::doRegister_()
for (const auto& crl : identity_.second->issuer->getRevocationLists())
dht_.put(h, crl, dht::DoneCallback{}, {}, true);
dht_.listen<DeviceAnnouncement>(h, [shared](DeviceAnnouncement&& dev) {
shared->findCertificate(dev.dev, [shared](const std::shared_ptr<dht::crypto::Certificate> crt) {
shared->findCertificate(dev.dev, [shared](const std::shared_ptr<dht::crypto::Certificate>& crt) {
shared->foundAccountDevice(crt);
});
return true;
......@@ -2014,7 +2014,7 @@ RingAccount::doRegister_()
if (v.service != DHT_TYPE_NS)
return true;
shared->findCertificate(v.from, [shared, v](const std::shared_ptr<dht::crypto::Certificate> cert) mutable {
shared->findCertificate(v.from, [shared, v](const std::shared_ptr<dht::crypto::Certificate>& cert) mutable {
auto& this_ = *shared.get();
// check peer certificate
......@@ -2140,7 +2140,7 @@ RingAccount::onPeerMessage(const dht::InfoHash& peer_device, std::function<void(
}
void
RingAccount::incomingCall(dht::IceCandidates&& msg, std::shared_ptr<dht::crypto::Certificate> from_cert)
RingAccount::incomingCall(dht::IceCandidates&& msg, const std::shared_ptr<dht::crypto::Certificate>& from_cert)
{
RING_WARN("ICE incoming from DHT peer %s", msg.from.toString().c_str());
auto call = Manager::instance().callFactory.newCall<SIPCall, RingAccount>(*this, Manager::instance().getNewCallID(), Call::CallType::INCOMING);
......@@ -2240,10 +2240,10 @@ RingAccount::foundPeerDevice(const std::shared_ptr<dht::crypto::Certificate>& cr
}
void
RingAccount::replyToIncomingIceMsg(std::shared_ptr<SIPCall> call,
std::shared_ptr<IceTransport> ice,
const dht::IceCandidates& peer_ice_msg,
std::shared_ptr<dht::crypto::Certificate> peer_cert)
RingAccount::replyToIncomingIceMsg(const std::shared_ptr<SIPCall>& call,
const std::shared_ptr<IceTransport>& ice,
const dht::IceCandidates& peer_ice_msg,
const std::shared_ptr<dht::crypto::Certificate>& peer_cert)
{
registerDhtAddress(*ice);
......@@ -2819,7 +2819,7 @@ RingAccount::onReceiveDeviceSync(DeviceSync&& sync)
RING_WARN("Received device sync data %zu", sync.devices_known.size());
for (const auto& d : sync.devices_known) {
auto shared = std::static_pointer_cast<RingAccount>(shared_from_this());
findCertificate(d.first, [shared,d](const std::shared_ptr<dht::crypto::Certificate> crt) {
findCertificate(d.first, [shared,d](const std::shared_ptr<dht::crypto::Certificate>& crt) {
if (not crt)
return;
shared->foundAccountDevice(crt, d.second);
......
......@@ -356,7 +356,7 @@ class RingAccount : public SIPAccountBase {
void onTrackedBuddyOnline(std::map<dht::InfoHash, BuddyInfo>::iterator& buddy_info_it, const dht::InfoHash& device_id);
void doRegister_();
void incomingCall(dht::IceCandidates&& msg, std::shared_ptr<dht::crypto::Certificate> from);
void incomingCall(dht::IceCandidates&& msg, const std::shared_ptr<dht::crypto::Certificate>& from);
const dht::ValueType USER_PROFILE_TYPE = {9, "User profile", std::chrono::hours(24 * 7)};
......@@ -497,10 +497,10 @@ class RingAccount : public SIPAccountBase {
void loadKnownDevices();
void saveKnownDevices() const;
void replyToIncomingIceMsg(std::shared_ptr<SIPCall>,
std::shared_ptr<IceTransport>,
void replyToIncomingIceMsg(const std::shared_ptr<SIPCall>&,
const std::shared_ptr<IceTransport>&,
const dht::IceCandidates&,
std::shared_ptr<dht::crypto::Certificate>);
const std::shared_ptr<dht::crypto::Certificate>&);
static tls::DhParams loadDhParams(const std::string path);
......
......@@ -142,7 +142,7 @@ CertificateStore::findCertificateByUID(const std::string& uid) const
}
std::shared_ptr<crypto::Certificate>
CertificateStore::findIssuer(std::shared_ptr<crypto::Certificate> crt) const
CertificateStore::findIssuer(const std::shared_ptr<crypto::Certificate>& crt) const
{
std::shared_ptr<crypto::Certificate> ret {};
auto n = crt->getIssuerUID();
......@@ -260,13 +260,13 @@ CertificateStore::pinCertificate(crypto::Certificate&& cert, bool local)
}
std::vector<std::string>
CertificateStore::pinCertificate(std::shared_ptr<crypto::Certificate> cert, bool local)
CertificateStore::pinCertificate(const std::shared_ptr<crypto::Certificate>& cert, bool local)
{
bool sig {false};
std::vector<std::string> ids {};
{
std::lock_guard<std::mutex> l(lock_);
auto c = cert;
std::lock_guard<std::mutex> l(lock_);
while (c) {
bool inserted;
auto id = c->getId().toString();
......
......@@ -61,11 +61,11 @@ public:
std::shared_ptr<crypto::Certificate> findCertificateByName(const std::string& name, crypto::Certificate::NameType type = crypto::Certificate::NameType::UNKNOWN) const;
std::shared_ptr<crypto::Certificate> findCertificateByUID(const std::string& uid) const;
std::shared_ptr<crypto::Certificate> findIssuer(std::shared_ptr<crypto::Certificate> crt) const;
std::shared_ptr<crypto::Certificate> findIssuer(const std::shared_ptr<crypto::Certificate>& crt) const;
std::vector<std::string> pinCertificate(const std::vector<uint8_t>& crt, bool local = true) noexcept;
std::vector<std::string> pinCertificate(crypto::Certificate&& crt, bool local = true);
std::vector<std::string> pinCertificate(std::shared_ptr<crypto::Certificate> crt, bool local = true);
std::vector<std::string> pinCertificate(const std::shared_ptr<crypto::Certificate>& crt, bool local = true);
bool unpinCertificate(const std::string&);
void pinCertificatePath(const std::string& path, std::function<void(const std::vector<std::string>&)> cb = {});
......
......@@ -152,7 +152,7 @@ private:
T creds_;
};
TlsSession::TlsSession(std::shared_ptr<IceTransport> ice, int ice_comp_id,
TlsSession::TlsSession(const std::shared_ptr<IceTransport>& ice, int ice_comp_id,
const TlsParams& params, const TlsSessionCallbacks& cbs, bool anonymous)
: socket_(new IceSocket(ice, ice_comp_id))
, isServer_(not ice->isInitiator())
......
......@@ -136,7 +136,7 @@ public:
VerifyCertificate verifyCertificate;
};
TlsSession(std::shared_ptr<IceTransport> ice, int ice_comp_id, const TlsParams& params,
TlsSession(const std::shared_ptr<IceTransport>& ice, int ice_comp_id, const TlsParams& params,
const TlsSessionCallbacks& cbs, bool anonymous=true);
~TlsSession();
......
......@@ -177,7 +177,7 @@ void SIPCall::setContactHeader(pj_str_t *contact)
}
void
SIPCall::setTransport(std::shared_ptr<SipTransport> t)
SIPCall::setTransport(const std::shared_ptr<SipTransport>& t)
{
if (isSecure() and t and not t->isSecure()) {
RING_ERR("Can't set unsecure transport to secure call.");
......@@ -1118,7 +1118,7 @@ SIPCall::initIceTransport(bool master, unsigned channel_num)
}
void
SIPCall::merge(std::shared_ptr<SIPCall> scall)
SIPCall::merge(const std::shared_ptr<SIPCall>& scall)
{
RING_WARN("SIPCall::merge %s -> %s", scall->getCallId().c_str(), getCallId().c_str());
inv = std::move(scall->inv);
......
......@@ -128,7 +128,7 @@ class SIPCall : public Call
void setContactHeader(pj_str_t *contact);
void setTransport(std::shared_ptr<SipTransport> t);
void setTransport(const std::shared_ptr<SipTransport>& t);
inline SipTransport* getTransport() {
return transport_.get();
......@@ -215,10 +215,10 @@ class SIPCall : public Call
void terminateSipSession(int status);
virtual void merge(std::shared_ptr<Call> scall) {
virtual void merge(const std::shared_ptr<Call>& scall) {
merge(std::dynamic_pointer_cast<SIPCall>(scall));
}
virtual void merge(std::shared_ptr<SIPCall> scall);
virtual void merge(const std::shared_ptr<SIPCall>& scall);
void setPeerRegistredName(const std::string& name) {
peerRegistredName_ = name;
......
......@@ -418,7 +418,7 @@ SipTransportBroker::getTlsTransport(const std::shared_ptr<TlsListener>& l, const
}
std::shared_ptr<SipTransport>
SipTransportBroker::getIceTransport(const std::shared_ptr<IceTransport> ice,
SipTransportBroker::getIceTransport(const std::shared_ptr<IceTransport>& ice,
unsigned comp_id)
{
auto sip_ice_tr = std::unique_ptr<SipIceTransport>(
......@@ -437,7 +437,7 @@ SipTransportBroker::getIceTransport(const std::shared_ptr<IceTransport> ice,
}
std::shared_ptr<SipTransport>
SipTransportBroker::getTlsIceTransport(const std::shared_ptr<ring::IceTransport> ice,
SipTransportBroker::getTlsIceTransport(const std::shared_ptr<ring::IceTransport>& ice,
unsigned comp_id,
const tls::TlsParams& params)
{
......
......@@ -176,10 +176,10 @@ public:
getTlsTransport(const std::shared_ptr<TlsListener>&, const IpAddr& remote, const std::string& remote_name = {});
std::shared_ptr<SipTransport>
getIceTransport(const std::shared_ptr<IceTransport>, unsigned comp_id);
getIceTransport(const std::shared_ptr<IceTransport>&, unsigned comp_id);
std::shared_ptr<SipTransport>
getTlsIceTransport(const std::shared_ptr<IceTransport>, unsigned comp_id,
getTlsIceTransport(const std::shared_ptr<IceTransport>&, unsigned comp_id,
const tls::TlsParams&);
std::shared_ptr<SipTransport> addTransport(pjsip_transport*);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment