From 0007bbfa11e74f2a76aebc83995843ee8f6840ab Mon Sep 17 00:00:00 2001 From: Tobias Markmann Date: Tue, 31 Jul 2018 17:42:11 +0200 Subject: Have StreamStack own intermediate layers via std::unique_ptr Test-Information: `./scons test=all` passes with no errors on macOS with clang 7 master. Change-Id: I31765ac15750dc5af7b70d1a85171dc8e3590181 diff --git a/Swiften/Session/BasicSessionStream.cpp b/Swiften/Session/BasicSessionStream.cpp index 54cd225..183b986 100644 --- a/Swiften/Session/BasicSessionStream.cpp +++ b/Swiften/Session/BasicSessionStream.cpp @@ -34,9 +34,6 @@ BasicSessionStream::BasicSessionStream( connection(connection), tlsContextFactory(tlsContextFactory), timerFactory(timerFactory), - compressionLayer(nullptr), - tlsLayer(nullptr), - whitespacePingLayer(nullptr), tlsOptions_(tlsOptions) { xmppLayer = new XMPPLayer(payloadParserFactories, payloadSerializers, xmlParserFactory, streamType); xmppLayer->onStreamStart.connect(boost::bind(&BasicSessionStream::handleStreamStartReceived, this, _1)); @@ -55,14 +52,11 @@ BasicSessionStream::BasicSessionStream( } BasicSessionStream::~BasicSessionStream() { - delete compressionLayer; - if (tlsLayer) { + if (auto tlsLayer = streamStack->getLayer()) { tlsLayer->onError.disconnect(boost::bind(&BasicSessionStream::handleTLSError, this, _1)); tlsLayer->onConnected.disconnect(boost::bind(&BasicSessionStream::handleTLSConnected, this)); - delete tlsLayer; } - delete whitespacePingLayer; delete streamStack; connection->onDisconnected.disconnect(boost::bind(&BasicSessionStream::handleConnectionFinished, this, _1)); @@ -112,12 +106,13 @@ bool BasicSessionStream::supportsTLSEncryption() { void BasicSessionStream::addTLSEncryption() { assert(available); auto tlsContext = tlsContextFactory->createTLSContext(tlsOptions_); - tlsLayer = new TLSLayer(std::move(tlsContext)); + auto tlsLayer = std::make_unique(std::move(tlsContext)); if (hasTLSCertificate() && !tlsLayer->setClientCertificate(getTLSCertificate())) { onClosed(std::make_shared(SessionStreamError::InvalidTLSCertificateError)); } else { - streamStack->addLayer(tlsLayer); + streamStack->addLayer(std::move(tlsLayer)); + auto tlsLayer = streamStack->getLayer(); tlsLayer->onError.connect(boost::bind(&BasicSessionStream::handleTLSError, this, _1)); tlsLayer->onConnected.connect(boost::bind(&BasicSessionStream::handleTLSConnected, this)); tlsLayer->connect(); @@ -125,23 +120,23 @@ void BasicSessionStream::addTLSEncryption() { } bool BasicSessionStream::isTLSEncrypted() { - return tlsLayer; + return streamStack->getLayer() != nullptr; } Certificate::ref BasicSessionStream::getPeerCertificate() const { - return tlsLayer->getPeerCertificate(); + return streamStack->getLayer()->getPeerCertificate(); } std::vector BasicSessionStream::getPeerCertificateChain() const { - return tlsLayer->getPeerCertificateChain(); + return streamStack->getLayer()->getPeerCertificateChain(); } std::shared_ptr BasicSessionStream::getPeerCertificateVerificationError() const { - return tlsLayer->getPeerCertificateVerificationError(); + return streamStack->getLayer()->getPeerCertificateVerificationError(); } ByteArray BasicSessionStream::getTLSFinishMessage() const { - return tlsLayer->getContext()->getFinishMessage(); + return streamStack->getLayer()->getContext()->getFinishMessage(); } bool BasicSessionStream::supportsZLibCompression() { @@ -149,15 +144,16 @@ bool BasicSessionStream::supportsZLibCompression() { } void BasicSessionStream::addZLibCompression() { - compressionLayer = new CompressionLayer(); - streamStack->addLayer(compressionLayer); + auto compressionLayer = std::make_unique(); + streamStack->addLayer(std::move(compressionLayer)); } void BasicSessionStream::setWhitespacePingEnabled(bool enabled) { + auto whitespacePingLayer = streamStack->getLayer(); if (enabled) { if (!whitespacePingLayer) { - whitespacePingLayer = new WhitespacePingLayer(timerFactory); - streamStack->addLayer(whitespacePingLayer); + streamStack->addLayer(std::make_unique(timerFactory)); + whitespacePingLayer = streamStack->getLayer(); } whitespacePingLayer->setActive(); } diff --git a/Swiften/Session/BasicSessionStream.h b/Swiften/Session/BasicSessionStream.h index 48b3d63..472b5cc 100644 --- a/Swiften/Session/BasicSessionStream.h +++ b/Swiften/Session/BasicSessionStream.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -85,9 +85,6 @@ namespace Swift { TimerFactory* timerFactory; XMPPLayer* xmppLayer; ConnectionLayer* connectionLayer; - CompressionLayer* compressionLayer; - TLSLayer* tlsLayer; - WhitespacePingLayer* whitespacePingLayer; StreamStack* streamStack; TLSOptions tlsOptions_; }; diff --git a/Swiften/StreamStack/StreamStack.cpp b/Swiften/StreamStack/StreamStack.cpp index 44a018d..f14f95c 100644 --- a/Swiften/StreamStack/StreamStack.cpp +++ b/Swiften/StreamStack/StreamStack.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -22,16 +22,16 @@ StreamStack::StreamStack(XMPPLayer* xmppLayer, LowLayer* physicalLayer) : xmppLa StreamStack::~StreamStack() { } -void StreamStack::addLayer(StreamLayer* newLayer) { - LowLayer* lowLayer = layers_.empty() ? physicalLayer_ : *layers_.rbegin(); +void StreamStack::addLayer(std::unique_ptr streamLayer) { + LowLayer* lowLayer = layers_.empty() ? physicalLayer_ : layers_.rbegin()->get(); - xmppLayer_->setChildLayer(newLayer); - newLayer->setParentLayer(xmppLayer_); + xmppLayer_->setChildLayer(streamLayer.get()); + streamLayer->setParentLayer(xmppLayer_); - lowLayer->setParentLayer(newLayer); - newLayer->setChildLayer(lowLayer); + lowLayer->setParentLayer(streamLayer.get()); + streamLayer->setChildLayer(lowLayer); - layers_.push_back(newLayer); + layers_.emplace_back(std::move(streamLayer)); } } diff --git a/Swiften/StreamStack/StreamStack.h b/Swiften/StreamStack/StreamStack.h index b12a69f..5b77085 100644 --- a/Swiften/StreamStack/StreamStack.h +++ b/Swiften/StreamStack/StreamStack.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -24,15 +24,15 @@ namespace Swift { StreamStack(XMPPLayer* xmppLayer, LowLayer* physicalLayer); ~StreamStack(); - void addLayer(StreamLayer*); + void addLayer(std::unique_ptr /* streamLayer */); XMPPLayer* getXMPPLayer() const { return xmppLayer_; } - template T* getLayer() { - for (auto& i : layers_) { - T* layer = dynamic_cast(i); + template T* getLayer() const { + for (const auto& i : layers_) { + T* layer = dynamic_cast(i.get()); if (layer) { return layer; } @@ -43,6 +43,6 @@ namespace Swift { private: XMPPLayer* xmppLayer_; LowLayer* physicalLayer_; - std::vector layers_; + std::vector> layers_; }; } diff --git a/Swiften/StreamStack/UnitTest/StreamStackTest.cpp b/Swiften/StreamStack/UnitTest/StreamStackTest.cpp index f0f82c9..0b520f1 100644 --- a/Swiften/StreamStack/UnitTest/StreamStackTest.cpp +++ b/Swiften/StreamStack/UnitTest/StreamStackTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -61,8 +61,8 @@ class StreamStackTest : public CppUnit::TestFixture { void testWriteData_OneIntermediateStream() { StreamStack testling(xmppStream_, physicalStream_); - std::shared_ptr xStream(new MyStreamLayer("X")); - testling.addLayer(xStream.get()); + std::unique_ptr xStream(new MyStreamLayer("X")); + testling.addLayer(std::move(xStream)); xmppStream_->writeData("foo"); @@ -72,10 +72,10 @@ class StreamStackTest : public CppUnit::TestFixture { void testWriteData_TwoIntermediateStreamStack() { StreamStack testling(xmppStream_, physicalStream_); - std::shared_ptr xStream(new MyStreamLayer("X")); - std::shared_ptr yStream(new MyStreamLayer("Y")); - testling.addLayer(xStream.get()); - testling.addLayer(yStream.get()); + std::unique_ptr xStream(new MyStreamLayer("X")); + std::unique_ptr yStream(new MyStreamLayer("Y")); + testling.addLayer(std::move(xStream)); + testling.addLayer(std::move(yStream)); xmppStream_->writeData("foo"); @@ -95,8 +95,8 @@ class StreamStackTest : public CppUnit::TestFixture { void testReadData_OneIntermediateStream() { StreamStack testling(xmppStream_, physicalStream_); xmppStream_->onElement.connect(boost::bind(&StreamStackTest::handleElement, this, _1)); - std::shared_ptr xStream(new MyStreamLayer("<")); - testling.addLayer(xStream.get()); + std::unique_ptr xStream(new MyStreamLayer("<")); + testling.addLayer(std::move(xStream)); physicalStream_->onDataRead(createSafeByteArray("stream:stream xmlns:stream='http://etherx.jabber.org/streams'>")); @@ -106,10 +106,10 @@ class StreamStackTest : public CppUnit::TestFixture { void testReadData_TwoIntermediateStreamStack() { StreamStack testling(xmppStream_, physicalStream_); xmppStream_->onElement.connect(boost::bind(&StreamStackTest::handleElement, this, _1)); - std::shared_ptr xStream(new MyStreamLayer("s")); - std::shared_ptr yStream(new MyStreamLayer("<")); - testling.addLayer(xStream.get()); - testling.addLayer(yStream.get()); + std::unique_ptr xStream(new MyStreamLayer("s")); + std::unique_ptr yStream(new MyStreamLayer("<")); + testling.addLayer(std::move(xStream)); + testling.addLayer(std::move(yStream)); physicalStream_->onDataRead(createSafeByteArray("tream:stream xmlns:stream='http://etherx.jabber.org/streams'>")); @@ -119,8 +119,8 @@ class StreamStackTest : public CppUnit::TestFixture { void testAddLayer_ExistingOnWriteDataSlot() { StreamStack testling(xmppStream_, physicalStream_); xmppStream_->onWriteData.connect(boost::bind(&StreamStackTest::handleWriteData, this, _1)); - std::shared_ptr xStream(new MyStreamLayer("X")); - testling.addLayer(xStream.get()); + std::unique_ptr xStream(new MyStreamLayer("X")); + testling.addLayer(std::move(xStream)); xmppStream_->writeData("foo"); -- cgit v0.10.2-6-g49f6