From ede42d5bb352841e2e425972e12b8ef31ddf2123 Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Sun, 11 Dec 2022 11:08:04 +0200 Subject: [PATCH] Use std::unique_ptr for owning pointers --- source/http/client.cpp | 48 +++++++++----------------- source/http/client.h | 13 +++---- source/http/server.cpp | 64 +++++++++++++++-------------------- source/http/server.h | 10 +++--- source/net/clientsocket.cpp | 2 -- source/net/clientsocket.h | 2 +- source/net/datagramsocket.cpp | 6 ++-- source/net/protocol.cpp | 19 +++-------- source/net/protocol.h | 32 ++++++------------ source/net/resolve.cpp | 19 ++++------- source/net/resolve.h | 7 ++-- source/net/socket.cpp | 12 +++---- source/net/socket.h | 7 ++-- source/net/streamsocket.cpp | 11 ++---- 14 files changed, 95 insertions(+), 157 deletions(-) diff --git a/source/http/client.cpp b/source/http/client.cpp index 30c336e..e8790aa 100644 --- a/source/http/client.cpp +++ b/source/http/client.cpp @@ -1,5 +1,4 @@ #include -#include #include #include #include "client.h" @@ -13,9 +12,6 @@ namespace Http { Client::~Client() { - delete sock; - delete request; - delete response; } void Client::use_event_dispatcher(IO::EventDispatcher *ed) @@ -30,14 +26,11 @@ void Client::use_event_dispatcher(IO::EventDispatcher *ed) void Client::use_resolver(Net::Resolver *r) { if(resolver) - { - delete resolve_listener; - resolve_listener = nullptr; - } + resolve_listener.reset(); resolver = r; if(resolver) - resolve_listener = new ResolveListener(*this); + resolve_listener = make_unique(*this); } void Client::start_request(const Request &r) @@ -45,15 +38,13 @@ void Client::start_request(const Request &r) if(request) throw invalid_state("already processing a request"); - delete sock; - sock = nullptr; + sock.reset(); - request = new Request(r); + request = make_unique(r); if(!user_agent.empty()) request->set_header("User-Agent", user_agent); - delete response; - response = nullptr; + response.reset(); in_buf.clear(); string host = r.get_header("Host"); @@ -63,7 +54,7 @@ void Client::start_request(const Request &r) resolve_tag = resolver->resolve(host); else { - RefPtr addr = Net::resolve(host); + unique_ptr addr(Net::resolve(host)); address_resolved(resolve_tag, *addr); } } @@ -72,7 +63,7 @@ const Response *Client::get_url(const std::string &url) { start_request(Request::from_url(url)); wait_response(); - return response; + return response.get(); } void Client::tick() @@ -87,10 +78,8 @@ void Client::tick() { signal_response_complete.emit(*response); - delete sock; - sock = nullptr; - delete request; - request = nullptr; + sock.reset(); + request.reset(); } } @@ -102,10 +91,8 @@ void Client::wait_response() void Client::abort() { - delete sock; - sock = nullptr; - delete request; - request = nullptr; + sock.reset(); + request.reset(); } void Client::address_resolved(unsigned tag, const Net::SockAddr &addr) @@ -114,7 +101,7 @@ void Client::address_resolved(unsigned tag, const Net::SockAddr &addr) return; resolve_tag = 0; - sock = new Net::StreamSocket(addr.get_family()); + sock = make_unique(addr.get_family()); sock->set_block(false); sock->signal_data_available.connect(sigc::mem_fun(this, &Client::data_available)); @@ -131,8 +118,7 @@ void Client::resolve_failed(unsigned tag, const exception &err) return; resolve_tag = 0; - delete request; - request = nullptr; + request.reset(); if(signal_socket_error.empty()) throw err; @@ -143,8 +129,7 @@ void Client::connect_finished(const exception *err) { if(err) { - delete request; - request = nullptr; + request.reset(); if(signal_socket_error.empty()) throw *err; @@ -190,7 +175,7 @@ void Client::data_available() { if(in_buf.find("\r\n\r\n")!=string::npos || in_buf.find("\n\n")!=string::npos) { - response = new Response(Response::parse(in_buf)); + response = make_unique(Response::parse(in_buf)); response->set_user_data(request->get_user_data()); in_buf = string(); } @@ -205,8 +190,7 @@ void Client::data_available() { signal_response_complete.emit(*response); - delete request; - request = nullptr; + request.reset(); } } diff --git a/source/http/client.h b/source/http/client.h index 2012498..d79389a 100644 --- a/source/http/client.h +++ b/source/http/client.h @@ -1,6 +1,7 @@ #ifndef MSP_HTTP_CLIENT_H_ #define MSP_HTTP_CLIENT_H_ +#include #include #include #include @@ -30,14 +31,14 @@ private: void resolve_failed(unsigned, const std::exception &); }; - Net::StreamSocket *sock = nullptr; + std::unique_ptr sock; IO::EventDispatcher *event_disp = nullptr; Net::Resolver *resolver = nullptr; - ResolveListener *resolve_listener = nullptr; + std::unique_ptr resolve_listener; unsigned resolve_tag = 0; std::string user_agent = "libmspnet/1.0"; - Request *request = nullptr; - Response *response = nullptr; + std::unique_ptr request; + std::unique_ptr response; std::string in_buf; Client(const Client &); @@ -53,8 +54,8 @@ public: void tick(); void wait_response(); void abort(); - const Request *get_request() const { return request; } - const Response *get_response() const { return response; } + const Request *get_request() const { return request.get(); } + const Response *get_response() const { return response.get(); } private: void address_resolved(unsigned, const Net::SockAddr &); void resolve_failed(unsigned, const std::exception &); diff --git a/source/http/server.cpp b/source/http/server.cpp index a7435d3..066fabe 100644 --- a/source/http/server.cpp +++ b/source/http/server.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include #include #include @@ -34,7 +33,7 @@ Server::~Server() void Server::listen(unsigned port) { - RefPtr addr = Net::resolve("*", format("%d", port), Net::INET6); + unique_ptr addr(Net::resolve("*", format("%d", port), Net::INET6)); sock.listen(*addr, 8); sock.signal_data_available.connect(sigc::mem_fun(this, &Server::data_available)); } @@ -105,12 +104,13 @@ void Server::close_connections(const Time::TimeDelta &timeout) void Server::data_available() { - Net::StreamSocket *csock = sock.accept(); - clients.push_back(Client(csock)); - csock->signal_data_available.connect(sigc::bind(sigc::mem_fun(this, &Server::client_data_available), sigc::ref(clients.back()))); - csock->signal_end_of_file.connect(sigc::bind(sigc::mem_fun(this, &Server::client_end_of_file), sigc::ref(clients.back()))); + unique_ptr csock(sock.accept()); + clients.emplace_back(move(csock)); + Client &cl = clients.back(); + cl.sock->signal_data_available.connect(sigc::bind(sigc::mem_fun(this, &Server::client_data_available), sigc::ref(clients.back()))); + cl.sock->signal_end_of_file.connect(sigc::bind(sigc::mem_fun(this, &Server::client_end_of_file), sigc::ref(clients.back()))); if(event_disp) - event_disp->add(*csock); + event_disp->add(*cl.sock); } void Server::client_data_available(Client &cl) @@ -136,14 +136,14 @@ void Server::client_data_available(Client &cl) return; } - RefPtr response; + unique_ptr response; if(!cl.request) { if(cl.in_buf.find("\r\n\r\n")!=string::npos || cl.in_buf.find("\n\n")!=string::npos) { try { - cl.request = new Request(Request::parse(cl.in_buf)); + cl.request = make_unique(Request::parse(cl.in_buf)); string addr_str = cl.sock->get_peer_address().str(); string::size_type colon = addr_str.find(':'); @@ -151,18 +151,18 @@ void Server::client_data_available(Client &cl) if(cl.request->get_method()!="GET" && cl.request->get_method()!="POST") { - response = new Response(NOT_IMPLEMENTED); + response = make_unique(NOT_IMPLEMENTED); response->add_content("Method not implemented\n"); } else if(cl.request->get_path()[0]!='/') { - response = new Response(BAD_REQUEST); + response = make_unique(BAD_REQUEST); response->add_content("Path must be absolute\n"); } } catch(const exception &e) { - response = new Response(BAD_REQUEST); + response = make_unique(BAD_REQUEST); response->add_content(format("An error occurred while parsing request headers:\ntype: %s\nwhat: %s", Debug::demangle(typeid(e).name()), e.what())); } @@ -181,30 +181,28 @@ void Server::client_data_available(Client &cl) if(cl.request->has_header("Connection")) cl.keepalive = !strcasecmp(cl.request->get_header("Connection"), "keep-alive"); - response = new Response(NONE); + response = make_unique(NONE); try { - cl.response = response.get(); - responses[cl.response] = &cl; - signal_request.emit(*cl.request, *response); - if(cl.async) - response.release(); - else + cl.response = move(response); + responses[cl.response.get()] = &cl; + signal_request.emit(*cl.request, *cl.response); + if(!cl.async) { - responses.erase(cl.response); - cl.response = nullptr; + responses.erase(cl.response.get()); + response = move(cl.response); if(response->get_status()==NONE) { - response = new Response(NOT_FOUND); + response = make_unique(NOT_FOUND); response->add_content("The requested resource was not found\n"); } } } catch(const exception &e) { - responses.erase(cl.response); - cl.response = nullptr; - response = new Response(INTERNAL_ERROR); + responses.erase(cl.response.get()); + cl.response.reset(); + response = make_unique(INTERNAL_ERROR); response->add_content(format("An error occurred while processing the request:\ntype: %s\nwhat: %s", Debug::demangle(typeid(e).name()), e.what())); } @@ -232,10 +230,8 @@ void Server::send_response(Client &cl, Response &resp) cl.async = false; if(cl.keepalive) { - delete cl.request; - cl.request = nullptr; - delete cl.response; - cl.response = nullptr; + cl.request.reset(); + cl.response.reset(); } else { @@ -255,15 +251,9 @@ Server::Client &Server::get_client_by_response(Response &resp) } -Server::Client::Client(RefPtr s): - sock(s) +Server::Client::Client(unique_ptr s): + sock(move(s)) { } -Server::Client::~Client() -{ - delete request; - delete response; -} - } // namespace Http } // namespace Msp diff --git a/source/http/server.h b/source/http/server.h index e6de227..dd8b2c6 100644 --- a/source/http/server.h +++ b/source/http/server.h @@ -1,7 +1,6 @@ #ifndef MSP_HTTP_SERVER_H_ #define MSP_HTTP_SERVER_H_ -#include #include #include #include @@ -20,16 +19,15 @@ public: private: struct Client { - RefPtr sock; + std::unique_ptr sock; std::string in_buf; - Request *request = nullptr; - Response *response = nullptr; + std::unique_ptr request; + std::unique_ptr response; bool keepalive = false; bool async = false; bool stale = false; - Client(RefPtr); - ~Client(); + Client(std::unique_ptr); }; Net::StreamServerSocket sock; diff --git a/source/net/clientsocket.cpp b/source/net/clientsocket.cpp index 89e2d08..e6e10e9 100644 --- a/source/net/clientsocket.cpp +++ b/source/net/clientsocket.cpp @@ -19,8 +19,6 @@ ClientSocket::ClientSocket(const Private &p, const SockAddr &paddr): ClientSocket::~ClientSocket() { signal_flush_required.emit(); - - delete peer_addr; } void ClientSocket::shutdown(IO::Mode m) diff --git a/source/net/clientsocket.h b/source/net/clientsocket.h index 2359fd3..c33ed93 100644 --- a/source/net/clientsocket.h +++ b/source/net/clientsocket.h @@ -18,7 +18,7 @@ public: protected: bool connecting = false; bool connected = false; - SockAddr *peer_addr = nullptr; + std::unique_ptr peer_addr; ClientSocket(const Private &, const SockAddr &); ClientSocket(Family, int, int); diff --git a/source/net/datagramsocket.cpp b/source/net/datagramsocket.cpp index e92e286..7ee057f 100644 --- a/source/net/datagramsocket.cpp +++ b/source/net/datagramsocket.cpp @@ -20,13 +20,11 @@ bool DatagramSocket::connect(const SockAddr &addr) SockAddr::SysAddr sa = addr.to_sys(); check_sys_connect_error(::connect(priv->handle, reinterpret_cast(&sa.addr), sa.size)); - delete peer_addr; - peer_addr = addr.copy(); + peer_addr.reset(addr.copy()); - delete local_addr; SockAddr::SysAddr lsa; getsockname(priv->handle, reinterpret_cast(&lsa.addr), &lsa.size); - local_addr = SockAddr::new_from_sys(lsa); + local_addr.reset(SockAddr::new_from_sys(lsa)); connected = true; diff --git a/source/net/protocol.cpp b/source/net/protocol.cpp index ce200a2..402b84a 100644 --- a/source/net/protocol.cpp +++ b/source/net/protocol.cpp @@ -18,29 +18,20 @@ Protocol::Protocol(unsigned npi): .fields(&PacketHeader::type, &PacketHeader::length); } -Protocol::~Protocol() -{ - for(auto &kvp: packet_class_defs) - delete kvp.second; -} - unsigned Protocol::get_next_packet_class_id() { static unsigned next_id = 1; return next_id++; } -void Protocol::add_packet(PacketDefBase *pdef) +void Protocol::add_packet(unique_ptr pdef) { - PacketDefBase *&ptr = packet_class_defs[pdef->get_class_id()]; + unique_ptr &ptr = packet_class_defs[pdef->get_class_id()]; if(ptr) - { packet_id_defs.erase(ptr->get_id()); - delete ptr; - } - ptr = pdef; - if(unsigned id = pdef->get_id()) - packet_id_defs[id] = pdef; + ptr = move(pdef); + if(unsigned id = ptr->get_id()) + packet_id_defs[id] = ptr.get(); } const Protocol::PacketDefBase &Protocol::get_packet_by_class_id(unsigned id) const diff --git a/source/net/protocol.h b/source/net/protocol.h index 35d3cb2..f040a69 100644 --- a/source/net/protocol.h +++ b/source/net/protocol.h @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -142,11 +143,10 @@ private: class PacketTypeDef: public PacketDefBase { private: - Serializer

*serializer; + std::unique_ptr> serializer; public: PacketTypeDef(unsigned); - ~PacketTypeDef(); unsigned get_class_id() const override { return get_packet_class_id

(); } @@ -188,17 +188,13 @@ private: PacketHeader(std::uint16_t, std::uint16_t); }; - typedef std::map PacketMap; - PacketTypeDef header_def; unsigned next_packet_id; - PacketMap packet_class_defs; - PacketMap packet_id_defs; + std::map> packet_class_defs; + std::map packet_id_defs; protected: Protocol(unsigned = 1); -public: - ~Protocol(); private: static unsigned get_next_packet_class_id(); @@ -206,7 +202,7 @@ private: template static unsigned get_packet_class_id(); - void add_packet(PacketDefBase *); + void add_packet(std::unique_ptr); protected: template @@ -248,9 +244,10 @@ unsigned Protocol::get_packet_class_id() template Protocol::PacketDefBuilder> Protocol::add(unsigned id) { - PacketTypeDef

*pdef = new PacketTypeDef

(id); - add_packet(pdef); - return PacketDefBuilder>(*this, *pdef, Serializer

()); + std::unique_ptr> pdef = std::make_unique>(id); + PacketDefBuilder> next(*this, *pdef, Serializer

()); + add_packet(move(pdef)); + return next; } template @@ -413,21 +410,14 @@ const char *Protocol::FieldSerializer::deserialize(C &com, const cha template Protocol::PacketTypeDef

::PacketTypeDef(unsigned i): PacketDefBase(i), - serializer(new Serializer

) + serializer(std::make_unique>()) { } -template -Protocol::PacketTypeDef

::~PacketTypeDef() -{ - delete serializer; -} - template template void Protocol::PacketTypeDef

::set_serializer(const S &ser) { - delete serializer; - serializer = new S(ser); + serializer = std::make_unique(ser); } template diff --git a/source/net/resolve.cpp b/source/net/resolve.cpp index e06bce7..078d6ef 100644 --- a/source/net/resolve.cpp +++ b/source/net/resolve.cpp @@ -102,7 +102,7 @@ unsigned Resolver::resolve(const string &host, const string &serv, Family family task.host = host; task.serv = serv; task.family = family; - thread.add_task(task); + thread.add_task(move(task)); return task.tag; } @@ -133,8 +133,7 @@ void Resolver::task_done() { if(signal_resolve_failed.empty()) { - RefPtr err = task->error; - task->error = nullptr; + unique_ptr err = move(task->error); thread.pop_complete_task(); throw *err; } @@ -159,11 +158,11 @@ Resolver::WorkerThread::~WorkerThread() join(); } -void Resolver::WorkerThread::add_task(const Task &t) +void Resolver::WorkerThread::add_task(Task &&t) { MutexLock lock(queue_mutex); bool was_starved = (queue.empty() || queue.back().is_complete()); - queue.push_back(t); + queue.push_back(move(t)); if(was_starved) sem.signal(); } @@ -181,11 +180,7 @@ void Resolver::WorkerThread::pop_complete_task() { MutexLock lock(queue_mutex); if(!queue.empty() && queue.front().is_complete()) - { - delete queue.front().addr; - delete queue.front().error; queue.pop_front(); - } } void Resolver::WorkerThread::main() @@ -209,16 +204,16 @@ void Resolver::WorkerThread::main() { try { - SockAddr *addr = Net::resolve(task->host, task->serv, task->family); + unique_ptr addr(Net::resolve(task->host, task->serv, task->family)); { MutexLock lock(queue_mutex); - task->addr = addr; + task->addr = move(addr); } } catch(const runtime_error &e) { MutexLock lock(queue_mutex); - task->error = new runtime_error(e); + task->error = make_unique(e); } notify_pipe.put(1); } diff --git a/source/net/resolve.h b/source/net/resolve.h index 65d1db8..ae0411b 100644 --- a/source/net/resolve.h +++ b/source/net/resolve.h @@ -2,6 +2,7 @@ #define MSP_NET_RESOLVE_H_ #include +#include #include #include #include @@ -40,8 +41,8 @@ private: std::string host; std::string serv; Family family = UNSPEC; - SockAddr *addr = nullptr; - std::runtime_error *error = nullptr; + std::unique_ptr addr; + std::unique_ptr error; bool is_complete() const { return addr || error; } }; @@ -59,7 +60,7 @@ private: WorkerThread(); ~WorkerThread(); - void add_task(const Task &); + void add_task(Task &&); Task *get_complete_task(); void pop_complete_task(); diff --git a/source/net/socket.cpp b/source/net/socket.cpp index 67bd381..877c1da 100644 --- a/source/net/socket.cpp +++ b/source/net/socket.cpp @@ -11,7 +11,7 @@ namespace Msp { namespace Net { Socket::Socket(const Private &p): - priv(new Private) + priv(make_unique()) { mode = IO::M_RDWR; @@ -19,13 +19,13 @@ Socket::Socket(const Private &p): SockAddr::SysAddr sa; getsockname(priv->handle, reinterpret_cast(&sa.addr), &sa.size); - local_addr = SockAddr::new_from_sys(sa); + local_addr.reset(SockAddr::new_from_sys(sa)); platform_init(); } Socket::Socket(Family af, int type, int proto): - priv(new Private) + priv(make_unique()) { mode = IO::M_RDWR; @@ -38,9 +38,6 @@ Socket::Socket(Family af, int type, int proto): Socket::~Socket() { platform_cleanup(); - - delete local_addr; - delete priv; } void Socket::set_block(bool b) @@ -74,8 +71,7 @@ void Socket::bind(const SockAddr &addr) if(err==-1) throw system_error("bind"); - delete local_addr; - local_addr = addr.copy(); + local_addr.reset(addr.copy()); } const SockAddr &Socket::get_local_address() const diff --git a/source/net/socket.h b/source/net/socket.h index c55a539..7a47793 100644 --- a/source/net/socket.h +++ b/source/net/socket.h @@ -1,6 +1,7 @@ #ifndef MSP_NET_SOCKET_H_ #define MSP_NET_SOCKET_H_ +#include #include #include #include @@ -30,8 +31,8 @@ protected: struct Private; - Private *priv = nullptr; - SockAddr *local_addr = nullptr; + std::unique_ptr priv; + std::unique_ptr local_addr; Socket(const Private &); Socket(Family, int, int); @@ -50,7 +51,7 @@ public: users of the address. */ void bind(const SockAddr &); - bool is_bound() const { return local_addr; } + bool is_bound() const { return static_cast(local_addr); } const SockAddr &get_local_address() const; void set_timeout(const Time::TimeDelta &); diff --git a/source/net/streamsocket.cpp b/source/net/streamsocket.cpp index c8eb1c4..02a7b54 100644 --- a/source/net/streamsocket.cpp +++ b/source/net/streamsocket.cpp @@ -34,13 +34,11 @@ bool StreamSocket::connect(const SockAddr &addr) set_socket_events(S_CONNECT); } - delete peer_addr; - peer_addr = addr.copy(); + peer_addr.reset(addr.copy()); - delete local_addr; SockAddr::SysAddr lsa; getsockname(priv->handle, reinterpret_cast(&lsa.addr), &lsa.size); - local_addr = SockAddr::new_from_sys(lsa); + local_addr.reset(SockAddr::new_from_sys(lsa)); if(finished) { @@ -99,10 +97,7 @@ void StreamSocket::on_event(IO::PollEvent ev) signal_connect_finished.emit(0); if(err!=0) - { - delete peer_addr; - peer_addr = nullptr; - } + peer_addr.reset(); set_socket_events((err==0) ? S_INPUT : S_NONE); } -- 2.43.0