From 6c07f223edeacdc6611cf747f2bc8701657e5dc6 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Wed, 13 Mar 2019 17:23:43 +0100 Subject: [PATCH] auth: Wrap existing pointers in smart pointers in the remotebackend --- modules/remotebackend/httpconnector.cc | 25 +++++++++---------------- modules/remotebackend/pipeconnector.cc | 21 ++++++++++----------- modules/remotebackend/remotebackend.cc | 18 +++++++----------- modules/remotebackend/remotebackend.hh | 10 +++++----- modules/remotebackend/zmqconnector.cc | 21 ++++++++------------- 5 files changed, 39 insertions(+), 56 deletions(-) diff --git a/modules/remotebackend/httpconnector.cc b/modules/remotebackend/httpconnector.cc index 7b2487eb1..2f2b13e7a 100644 --- a/modules/remotebackend/httpconnector.cc +++ b/modules/remotebackend/httpconnector.cc @@ -34,7 +34,7 @@ #define UNIX_PATH_MAX 108 #endif -HTTPConnector::HTTPConnector(std::map options) { +HTTPConnector::HTTPConnector(std::map options): d_socket(nullptr) { this->d_url = options.find("url")->second; if (options.find("url-suffix") != options.end()) { this->d_url_suffix = options.find("url-suffix")->second; @@ -44,7 +44,6 @@ HTTPConnector::HTTPConnector(std::map options) { this->timeout = 2; this->d_post = false; this->d_post_json = false; - this->d_socket = NULL; if (options.find("timeout") != options.end()) { this->timeout = std::stoi(options.find("timeout")->second)/1000; @@ -63,10 +62,7 @@ HTTPConnector::HTTPConnector(std::map options) { } } -HTTPConnector::~HTTPConnector() { - if (d_socket != NULL) - delete d_socket; -} +HTTPConnector::~HTTPConnector() { } void HTTPConnector::addUrlComponent(const Json ¶meters, const string& element, std::stringstream& ss) { std::string sparam; @@ -305,7 +301,7 @@ int HTTPConnector::send_message(const Json& input) { out << req; // try sending with current socket, if it fails retry with new socket - if (this->d_socket != NULL) { + if (this->d_socket != nullptr) { fd = this->d_socket->getHandle(); // there should be no data waiting if (waitForRWData(fd, true, 0, 1000) < 1) { @@ -322,8 +318,7 @@ int HTTPConnector::send_message(const Json& input) { if (rv == 1) return rv; - delete this->d_socket; - this->d_socket = NULL; + this->d_socket.reset(); if (req.url.protocol == "unix") { // connect using unix socket @@ -342,7 +337,7 @@ int HTTPConnector::send_message(const Json& input) { while(gAddrPtr) { try { - d_socket = new Socket(gAddrPtr->ai_family, gAddrPtr->ai_socktype, gAddrPtr->ai_protocol); + d_socket = std::unique_ptr(new Socket(gAddrPtr->ai_family, gAddrPtr->ai_socktype, gAddrPtr->ai_protocol)); d_addr.setSockaddr(gAddrPtr->ai_addr, gAddrPtr->ai_addrlen); d_socket->connect(d_addr); d_socket->setNonBlocking(); @@ -355,8 +350,7 @@ int HTTPConnector::send_message(const Json& input) { } if (rv > -1) break; - delete d_socket; - d_socket = NULL; + d_socket.reset(); gAddrPtr = gAddrPtr->ai_next; } @@ -373,7 +367,7 @@ int HTTPConnector::recv_message(Json& output) { YaHTTP::AsyncResponseLoader arl; YaHTTP::Response resp; - if (d_socket == NULL ) return -1; // cannot receive :( + if (d_socket == nullptr ) return -1; // cannot receive :( char buffer[4096]; int rd = -1; bool fail = false; @@ -396,12 +390,11 @@ int HTTPConnector::recv_message(Json& output) { throw NetworkError("timeout"); } catch (NetworkError &ne) { g_log< optionsMap) { +PipeConnector::PipeConnector(std::map optionsMap): d_pid(-1) { if (optionsMap.count("command") == 0) { g_log< optionsMap) { d_timeout = std::stoi(optionsMap.find("timeout")->second); } - d_pid = -1; - d_fp = NULL; d_fd1[0] = d_fd1[1] = -1; d_fd2[0] = d_fd2[1] = -1; } @@ -53,8 +51,9 @@ PipeConnector::~PipeConnector(){ waitpid(d_pid, &status, 0); } - close(d_fd1[1]); - if (d_fp != NULL) fclose(d_fp); + if (d_fd1[1]) { + close(d_fd1[1]); + } } void PipeConnector::launch() { @@ -85,10 +84,10 @@ void PipeConnector::launch() { setCloseOnExec(d_fd1[1]); close(d_fd2[1]); setCloseOnExec(d_fd2[0]); - if(!(d_fp=fdopen(d_fd2[0],"r"))) + if(!(d_fp=std::unique_ptr(fdopen(d_fd2[0],"r"), fclose))) throw PDNSException("Unable to associate a file pointer with pipe: "+stringerror()); - if (d_timeout) - setbuf(d_fp,0); // no buffering please, confuses select + if (d_timeout) + setbuf(d_fp.get(),0); // no buffering please, confuses select } else if(!d_pid) { // child signal(SIGCHLD, SIG_DFL); // silence a warning from perl @@ -163,15 +162,15 @@ int PipeConnector::recv_message(Json& output) tv.tv_usec = (d_timeout % 1000) * 1000; fd_set rds; FD_ZERO(&rds); - FD_SET(fileno(d_fp),&rds); - int ret=select(fileno(d_fp)+1,&rds,0,0,&tv); + FD_SET(fileno(d_fp.get()),&rds); + int ret=select(fileno(d_fp.get())+1,&rds,0,0,&tv); if(ret<0) throw PDNSException("Error waiting on data from coprocess: "+stringerror()); if(!ret) throw PDNSException("Timeout waiting for data from coprocess"); } - if(!stringfgets(d_fp, receive)) + if(!stringfgets(d_fp.get(), receive)) throw PDNSException("Child closed pipe"); s_output.append(receive); diff --git a/modules/remotebackend/remotebackend.cc b/modules/remotebackend/remotebackend.cc index f3a351572..c77728a68 100644 --- a/modules/remotebackend/remotebackend.cc +++ b/modules/remotebackend/remotebackend.cc @@ -71,11 +71,7 @@ RemoteBackend::RemoteBackend(const std::string &suffix) build(); } -RemoteBackend::~RemoteBackend() { - if (connector != NULL) { - delete connector; - } -} +RemoteBackend::~RemoteBackend() { } bool RemoteBackend::send(Json& value) { try { @@ -84,7 +80,7 @@ bool RemoteBackend::send(Json& value) { g_log<connector; + this->connector.reset(); build(); return false; } @@ -98,7 +94,7 @@ bool RemoteBackend::recv(Json& value) { g_log<connector; + this->connector.reset(); build(); return false; } @@ -147,17 +143,17 @@ int RemoteBackend::build() { // connectors know what they are doing if (type == "unix") { - this->connector = new UnixsocketConnector(options); + this->connector = std::unique_ptr(new UnixsocketConnector(options)); } else if (type == "http") { - this->connector = new HTTPConnector(options); + this->connector = std::unique_ptr(new HTTPConnector(options)); } else if (type == "zeromq") { #ifdef REMOTEBACKEND_ZEROMQ - this->connector = new ZeroMQConnector(options); + this->connector = std::unique_ptr(new ZeroMQConnector(options)); #else throw PDNSException("Invalid connection string: zeromq connector support not enabled. Recompile with --enable-remotebackend-zeromq"); #endif } else if (type == "pipe") { - this->connector = new PipeConnector(options); + this->connector = std::unique_ptr(new PipeConnector(options)); } else { throw PDNSException("Invalid connection string: unknown connector"); } diff --git a/modules/remotebackend/remotebackend.hh b/modules/remotebackend/remotebackend.hh index 0c64a7dee..2b109a0a5 100644 --- a/modules/remotebackend/remotebackend.hh +++ b/modules/remotebackend/remotebackend.hh @@ -102,7 +102,7 @@ class HTTPConnector: public Connector { void post_requestbuilder(const Json &input, YaHTTP::Request& req); void addUrlComponent(const Json ¶meters, const string& element, std::stringstream& ss); std::string buildMemberListArgs(std::string prefix, const Json& args); - Socket* d_socket; + std::unique_ptr d_socket; ComboAddress d_addr; }; @@ -119,8 +119,8 @@ class ZeroMQConnector: public Connector { int d_timeout; int d_timespent; std::map d_options; - void *d_ctx; - void *d_sock; + std::unique_ptr d_ctx; + std::unique_ptr d_sock; }; #endif @@ -144,7 +144,7 @@ class PipeConnector: public Connector { int d_fd1[2], d_fd2[2]; int d_pid; int d_timeout; - FILE *d_fp; + std::unique_ptr d_fp{nullptr, fclose}; }; class RemoteBackend : public DNSBackend @@ -192,7 +192,7 @@ class RemoteBackend : public DNSBackend private: int build(); - Connector *connector; + std::unique_ptr connector; bool d_dnssec; Json d_result; int d_index; diff --git a/modules/remotebackend/zmqconnector.cc b/modules/remotebackend/zmqconnector.cc index a525d3365..1c5ae093f 100644 --- a/modules/remotebackend/zmqconnector.cc +++ b/modules/remotebackend/zmqconnector.cc @@ -25,7 +25,7 @@ #include "remotebackend.hh" #ifdef REMOTEBACKEND_ZEROMQ -ZeroMQConnector::ZeroMQConnector(std::map options) { +ZeroMQConnector::ZeroMQConnector(std::map options): d_ctx(std::unique_ptr(zmq_init(2), zmq_close)), d_sock(std::unique_ptr(zmq_socket(d_ctx.get(), ZMQ_REQ), zmq_close)) { int opt=0; // lookup timeout, target and stuff @@ -41,11 +41,9 @@ ZeroMQConnector::ZeroMQConnector(std::map options) { this->d_timeout = std::stoi(options.find("timeout")->second); } - d_ctx = zmq_init(2); - d_sock = zmq_socket(this->d_ctx, ZMQ_REQ); - zmq_setsockopt(d_sock, ZMQ_LINGER, &opt, sizeof(opt)); + zmq_setsockopt(d_sock.get(), ZMQ_LINGER, &opt, sizeof(opt)); - if(zmq_connect(this->d_sock, this->d_endpoint.c_str()) < 0) + if(zmq_connect(this->d_sock.get(), this->d_endpoint.c_str()) < 0) { g_log< options) { } }; -ZeroMQConnector::~ZeroMQConnector() { - zmq_close(this->d_sock); - zmq_term(this->d_ctx); -}; +ZeroMQConnector::~ZeroMQConnector() {} int ZeroMQConnector::send_message(const Json& input) { auto line = input.dump(); @@ -80,13 +75,13 @@ int ZeroMQConnector::send_message(const Json& input) { try { zmq_pollitem_t item; - item.socket = d_sock; + item.socket = d_sock.get(); item.events = ZMQ_POLLOUT; // poll until it's sent or timeout is spent. try to leave // leave few cycles for read. just in case. for(d_timespent = 0; d_timespent < d_timeout-5; d_timespent++) { if (zmq_poll(&item, 1, 1)>0) { - if(zmq_msg_send(&message, this->d_sock, 0) == -1) { + if(zmq_msg_send(&message, this->d_sock.get(), 0) == -1) { // message was not sent g_log<d_endpoint << ": " << zmq_strerror(errno)<d_sock, ZMQ_NOBLOCK)>0) { + if(zmq_msg_recv(&message, this->d_sock.get(), ZMQ_NOBLOCK)>0) { string err; msg_size = zmq_msg_size(&message); data.assign(reinterpret_cast(zmq_msg_data(&message)), msg_size); -- 2.40.0