From 52a9285ca78cb378c1b3c4487049a2d867a45d63 Mon Sep 17 00:00:00 2001 From: "Jannis M. Hoffmann" Date: Wed, 11 Oct 2023 23:26:51 +0200 Subject: use smart pointer types in IOFactory, BrokerFactory and Broker --- src/broker.cc | 40 +++++++++-------------- src/include/broker.h | 36 ++++++++++----------- src/include/imapparser.h | 12 +++---- src/include/iofactory.h | 12 +++---- src/iofactory.cc | 16 ++------- src/session-initialize-bincimap-up.cc | 34 +++++++++---------- src/session-initialize-bincimapd.cc | 61 ++++++++++++++++++----------------- 7 files changed, 91 insertions(+), 120 deletions(-) diff --git a/src/broker.cc b/src/broker.cc index 3270702..59c23bb 100644 --- a/src/broker.cc +++ b/src/broker.cc @@ -14,18 +14,11 @@ using std::string; BrokerFactory::BrokerFactory() { - auto &session = Session::getInstance(); - Parser p(session); + Parser p{Session::getInstance()}; - brokers[Session::State::NONAUTHENTICATED] = new Broker(p); - brokers[Session::State::AUTHENTICATED] = new Broker(p); - brokers[Session::State::SELECTED] = new Broker(p); -} - -BrokerFactory::~BrokerFactory() -{ - for (auto &[_, second] : brokers) - delete second; + brokers.emplace(Session::State::NONAUTHENTICATED, Broker(p)); + brokers.emplace(Session::State::AUTHENTICATED, Broker(p)); + brokers.emplace(Session::State::SELECTED, Broker(p)); } BrokerFactory &BrokerFactory::getInstance() @@ -34,10 +27,10 @@ BrokerFactory &BrokerFactory::getInstance() return brokerfactory; } -void BrokerFactory::addCapability(const std::string &c) +void BrokerFactory::addCapability(const string &c) { for (const auto &[_, second] : brokers) { - auto o = dynamic_cast(second->get("CAPABILITY")); + auto o = dynamic_cast(second.get("CAPABILITY")); if (o != nullptr) { o->addCapability(c); break; @@ -45,13 +38,11 @@ void BrokerFactory::addCapability(const std::string &c) } } -void BrokerFactory::assign(const string &fname, Operator *o) +void BrokerFactory::assign(const string &fname, std::shared_ptr o) { - int deletable = true; - for (const auto &[first, second] : brokers) { - if (first & o->getState()) { - second->assign(fname, o, deletable); - deletable = false; + for (auto &[state, broker] : brokers) { + if (state & o->getState()) { + broker.assign(fname, o); } } } @@ -61,30 +52,29 @@ Operator *BrokerFactory::getOperator(Session::State state, const string &name) c if (brokers.find(state) == brokers.end()) return nullptr; else - return brokers.find(state)->second->get(name); + return brokers.find(state)->second.get(name); } Broker *BrokerFactory::getBroker(Session::State state) { - auto it = brokers.find(state); + const auto it = brokers.find(state); if (it == brokers.end()) { setLastError("No appropriate broker for state."); return nullptr; } - return it->second; + return &it->second; } -void Broker::assign(const string &fname, Operator *o, bool deletable) +void Broker::assign(const string &fname, std::shared_ptr o) { - deletables[fname] = deletable; operators[fname] = o; } Operator *Broker::get(const string &name) const { if (operators.find(name) == operators.end()) return nullptr; - return operators.find(name)->second; + return operators.find(name)->second.get(); } Parser::ParseResult Broker::parseStub(Request &command) diff --git a/src/include/broker.h b/src/include/broker.h index 57c79dc..a6428e1 100644 --- a/src/include/broker.h +++ b/src/include/broker.h @@ -14,6 +14,7 @@ #include "session.h" #include +#include #include namespace Binc { @@ -23,7 +24,7 @@ namespace Binc { class BrokerFactory { private: - std::map brokers; + std::map brokers; BrokerFactory(); @@ -31,40 +32,39 @@ namespace Binc { public: Broker *getBroker(Session::State state); - void assign(const std::string &fname, Operator *o); + void assign(const std::string &fname, std::shared_ptr o); void addCapability(const std::string &c); Operator *getOperator(Session::State state, const std::string &name) const; - inline const std::string &getLastError() const; - inline void setLastError(const std::string &error) const; + const std::string &getLastError() const + { + return lastError; + } - static BrokerFactory &getInstance(); - ~BrokerFactory(); - }; + void setLastError(std::string error) const + { + lastError = std::move(error); + } - const std::string &BrokerFactory::getLastError() const - { - return lastError; - } + static BrokerFactory &getInstance(); - void BrokerFactory::setLastError(const std::string &error) const - { - lastError = error; - } + BrokerFactory(const BrokerFactory &) = delete; + BrokerFactory(const BrokerFactory &&) = delete; + }; class Broker { private: - std::map operators; - std::map deletables; + std::map> operators; public: Parser &parser; Operator *get(const std::string &name) const; - void assign(const std::string &fname, Operator *o, bool deletable = false); + void assign(const std::string &fname, std::shared_ptr o); Parser::ParseResult parseStub(Request &cmd); Broker(Parser &p) : parser(p){}; + Broker(const Broker &) = delete; Broker(Broker &&) = default; }; diff --git a/src/include/imapparser.h b/src/include/imapparser.h index e0daeb2..7d2ac27 100644 --- a/src/include/imapparser.h +++ b/src/include/imapparser.h @@ -28,10 +28,9 @@ namespace Binc { static SequenceSet &null(); - SequenceSet &operator=(const SequenceSet ©); - SequenceSet(); SequenceSet(const SequenceSet ©); + SequenceSet &operator=(const SequenceSet ©); ~SequenceSet(); protected: @@ -66,8 +65,7 @@ namespace Binc { std::string toString(); }; - class BincImapParserSearchKey { - public: + struct BincImapParserSearchKey { std::string name; std::string date; std::string astring; @@ -85,13 +83,11 @@ namespace Binc { BincImapParserSearchKey(); }; - class BincImapParserData { - public: - virtual ~BincImapParserData() {} + struct BincImapParserData { + virtual ~BincImapParserData() = default; }; class Request { - private: std::string tag; std::string name; std::string mode; diff --git a/src/include/iofactory.h b/src/include/iofactory.h index 24dfe9f..e19688a 100644 --- a/src/include/iofactory.h +++ b/src/include/iofactory.h @@ -11,22 +11,18 @@ #include "iodevice.h" #include +#include #include namespace Binc { class IOFactory { - public: - ~IOFactory(); + std::map> devices; - static void addDevice(IODevice *dev); + public: + static void addDevice(std::unique_ptr dev); static IOFactory &getInstance(); static IODevice &getClient(); static IODevice &getLogger(); - - private: - IOFactory(); - - std::map devices; }; } diff --git a/src/iofactory.cc b/src/iofactory.cc index 2fe4816..2a50ce1 100644 --- a/src/iofactory.cc +++ b/src/iofactory.cc @@ -7,29 +7,17 @@ #include "iofactory.h" -#include "iodevice.h" - using namespace Binc; -IOFactory::IOFactory() {} - -IOFactory::~IOFactory() {} - IOFactory &IOFactory::getInstance() { static IOFactory ioFactory; return ioFactory; } -void IOFactory::addDevice(IODevice *dev) +void IOFactory::addDevice(std::unique_ptr dev) { - IODevice *ioDevice = IOFactory::getInstance().devices[dev->service()]; - - // FIXME: Delete correct object. Now, only IODevice's destructor is - // called, and only IODevice's memory is freed. - if (ioDevice) delete ioDevice; - - IOFactory::getInstance().devices[dev->service()] = dev; + IOFactory::getInstance().devices[dev->service()] = std::move(dev); } IODevice &IOFactory::getClient() diff --git a/src/session-initialize-bincimap-up.cc b/src/session-initialize-bincimap-up.cc index ee9f8db..88dc788 100644 --- a/src/session-initialize-bincimap-up.cc +++ b/src/session-initialize-bincimap-up.cc @@ -17,7 +17,6 @@ #include "syslogdevice.h" #include "tools.h" -#include #include #include #include @@ -32,10 +31,10 @@ bool Session::initialize(int argc, char *argv[]) { IOFactory &ioFactory = IOFactory::getInstance(); - IODevice *stdioDevice = new StdIODevice(IODevice::IsEnabled); + auto stdioDevice = new StdIODevice(IODevice::IsEnabled); stdioDevice->setFlags(IODevice::HasOutputLimit); stdioDevice->setMaxOutputBufferSize(TRANSFER_BUFFER_SIZE); - ioFactory.addDevice(stdioDevice); + ioFactory.addDevice(std::unique_ptr(stdioDevice)); Session &session = Session::getInstance(); @@ -68,8 +67,8 @@ bool Session::initialize(int argc, char *argv[]) lowercase(logtype); trim(logtype); if (logtype == "" || logtype == "multilog") { - MultilogDevice *device = new MultilogDevice(IODevice::IsEnabled | IODevice::FlushesOnEndl); - ioFactory.addDevice(device); + auto device = new MultilogDevice(IODevice::IsEnabled | IODevice::FlushesOnEndl); + ioFactory.addDevice(std::unique_ptr(device)); } else if (logtype == "syslog") { const string f = session.getEnv("SYSLOG_FACILITY"); int facility = 0; @@ -98,11 +97,11 @@ bool Session::initialize(int argc, char *argv[]) facility = LOG_DAEMON; } - SyslogDevice *device = new SyslogDevice(IODevice::IsEnabled | IODevice::FlushesOnEndl, - "bincimap-up", - LOG_NDELAY | LOG_PID, - facility); - ioFactory.addDevice(device); + auto device = new SyslogDevice(IODevice::IsEnabled | IODevice::FlushesOnEndl, + "bincimap-up", + LOG_NDELAY | LOG_PID, + facility); + ioFactory.addDevice(std::unique_ptr(device)); } // Now that we know the log type, we can flush. @@ -125,13 +124,14 @@ bool Session::initialize(int argc, char *argv[]) BrokerFactory &brokerfactory = BrokerFactory::getInstance(); - brokerfactory.assign("AUTHENTICATE", new AuthenticateOperator()); - brokerfactory.assign("CAPABILITY", new CapabilityOperator()); - brokerfactory.assign("LOGIN", new LoginOperator()); - brokerfactory.assign("LOGOUT", new LogoutOperator()); - brokerfactory.assign("NOOP", new NoopOperator()); - brokerfactory.assign("ID", new IdOperator()); - if (stls > 0) brokerfactory.assign("STARTTLS", new StarttlsOperator()); + brokerfactory.assign("AUTHENTICATE", std::shared_ptr(new AuthenticateOperator())); + brokerfactory.assign("CAPABILITY", std::shared_ptr(new CapabilityOperator())); + brokerfactory.assign("LOGIN", std::shared_ptr(new LoginOperator())); + brokerfactory.assign("LOGOUT", std::shared_ptr(new LogoutOperator())); + brokerfactory.assign("NOOP", std::shared_ptr(new NoopOperator())); + brokerfactory.assign("ID", std::shared_ptr(new IdOperator())); + if (stls > 0) + brokerfactory.assign("STARTTLS", std::shared_ptr(new StarttlsOperator())); bincClient.setTimeout(AUTH_TIMEOUT); diff --git a/src/session-initialize-bincimapd.cc b/src/session-initialize-bincimapd.cc index 60543b5..27ad2d5 100644 --- a/src/session-initialize-bincimapd.cc +++ b/src/session-initialize-bincimapd.cc @@ -22,6 +22,7 @@ #include #include +#include #include using namespace Binc; @@ -32,11 +33,11 @@ bool Session::initialize(int argc, char *argv[]) { IOFactory &ioFactory = IOFactory::getInstance(); - IODevice *stdioDevice = new StdIODevice(IODevice::IsEnabled | IODevice::HasInputLimit - | IODevice::HasTimeout); + auto stdioDevice = new StdIODevice(IODevice::IsEnabled | IODevice::HasInputLimit + | IODevice::HasTimeout); stdioDevice->setFlags(IODevice::HasOutputLimit); stdioDevice->setMaxOutputBufferSize(TRANSFER_BUFFER_SIZE); - ioFactory.addDevice(stdioDevice); + ioFactory.addDevice(std::unique_ptr(stdioDevice)); Session &session = Session::getInstance(); @@ -67,7 +68,7 @@ bool Session::initialize(int argc, char *argv[]) lowercase(logtype); trim(logtype); if (logtype == "" || logtype == "multilog") { - ioFactory.addDevice(new MultilogDevice(IODevice::IsEnabled)); + ioFactory.addDevice(std::unique_ptr(new MultilogDevice(IODevice::IsEnabled))); } else if (logtype == "syslog") { const string f = session.getEnv("SYSLOG_FACILITY"); @@ -99,8 +100,8 @@ bool Session::initialize(int argc, char *argv[]) session.setEnv("SYSLOG_FACILITY", std::to_string(facility)); - ioFactory.addDevice( - new SyslogDevice(IODevice::IsEnabled, "bincimapd", LOG_NDELAY | LOG_PID, facility)); + ioFactory.addDevice(std::unique_ptr( + new SyslogDevice(IODevice::IsEnabled, "bincimapd", LOG_NDELAY | LOG_PID, facility))); } // Now that we know the log type, we can flush. @@ -141,30 +142,30 @@ bool Session::initialize(int argc, char *argv[]) BrokerFactory &brokerfactory = BrokerFactory::getInstance(); - brokerfactory.assign("APPEND", new AppendOperator()); - brokerfactory.assign("CAPABILITY", new CapabilityOperator()); - brokerfactory.assign("CHECK", new CheckOperator()); - brokerfactory.assign("CLOSE", new CloseOperator()); - brokerfactory.assign("COPY", new CopyOperator()); - brokerfactory.assign("CREATE", new CreateOperator()); - brokerfactory.assign("DELETE", new DeleteOperator()); - brokerfactory.assign("EXAMINE", new ExamineOperator()); - brokerfactory.assign("EXPUNGE", new ExpungeOperator()); - brokerfactory.assign("FETCH", new FetchOperator()); - brokerfactory.assign("IDLE", new IdleOperator()); - brokerfactory.assign("ID", new IdOperator()); - brokerfactory.assign("LIST", new ListOperator()); - brokerfactory.assign("LOGOUT", new LogoutOperator()); - brokerfactory.assign("LSUB", new LsubOperator()); - brokerfactory.assign("NAMESPACE", new NamespaceOperator()); - brokerfactory.assign("NOOP", new NoopPendingOperator()); - brokerfactory.assign("RENAME", new RenameOperator()); - brokerfactory.assign("SEARCH", new SearchOperator()); - brokerfactory.assign("SELECT", new SelectOperator()); - brokerfactory.assign("STATUS", new StatusOperator()); - brokerfactory.assign("STORE", new StoreOperator()); - brokerfactory.assign("SUBSCRIBE", new SubscribeOperator()); - brokerfactory.assign("UNSUBSCRIBE", new UnsubscribeOperator()); + brokerfactory.assign("APPEND", std::shared_ptr(new AppendOperator())); + brokerfactory.assign("CAPABILITY", std::shared_ptr(new CapabilityOperator())); + brokerfactory.assign("CHECK", std::shared_ptr(new CheckOperator())); + brokerfactory.assign("CLOSE", std::shared_ptr(new CloseOperator())); + brokerfactory.assign("COPY", std::shared_ptr(new CopyOperator())); + brokerfactory.assign("CREATE", std::shared_ptr(new CreateOperator())); + brokerfactory.assign("DELETE", std::shared_ptr(new DeleteOperator())); + brokerfactory.assign("EXAMINE", std::shared_ptr(new ExamineOperator())); + brokerfactory.assign("EXPUNGE", std::shared_ptr(new ExpungeOperator())); + brokerfactory.assign("FETCH", std::shared_ptr(new FetchOperator())); + brokerfactory.assign("IDLE", std::shared_ptr(new IdleOperator())); + brokerfactory.assign("ID", std::shared_ptr(new IdOperator())); + brokerfactory.assign("LIST", std::shared_ptr(new ListOperator())); + brokerfactory.assign("LOGOUT", std::shared_ptr(new LogoutOperator())); + brokerfactory.assign("LSUB", std::shared_ptr(new LsubOperator())); + brokerfactory.assign("NAMESPACE", std::shared_ptr(new NamespaceOperator())); + brokerfactory.assign("NOOP", std::shared_ptr(new NoopPendingOperator())); + brokerfactory.assign("RENAME", std::shared_ptr(new RenameOperator())); + brokerfactory.assign("SEARCH", std::shared_ptr(new SearchOperator())); + brokerfactory.assign("SELECT", std::shared_ptr(new SelectOperator())); + brokerfactory.assign("STATUS", std::shared_ptr(new StatusOperator())); + brokerfactory.assign("STORE", std::shared_ptr(new StoreOperator())); + brokerfactory.assign("SUBSCRIBE", std::shared_ptr(new SubscribeOperator())); + brokerfactory.assign("UNSUBSCRIBE", std::shared_ptr(new UnsubscribeOperator())); // automatically create depot directory if it's not there already string path; -- cgit v1.2.3