Skip to content

Commit

Permalink
auth: Get rid of most remaining naked pointers
Browse files Browse the repository at this point in the history
  • Loading branch information
rgacogne committed Sep 25, 2019
1 parent c604f94 commit c2826d2
Show file tree
Hide file tree
Showing 54 changed files with 635 additions and 724 deletions.
1 change: 0 additions & 1 deletion modules/bindbackend/bindbackend2.hh
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,6 @@ private:
void setupDNSSEC();
void setupStatements();
void freeStatements();
void release(SSqlStatement**);
static bool safeGetBBDomainInfo(int id, BB2DomainInfo* bbd);
static void safePutBBDomainInfo(const BB2DomainInfo& bbd);
static bool safeGetBBDomainInfo(const DNSName& name, BB2DomainInfo* bbd);
Expand Down
5 changes: 0 additions & 5 deletions modules/bindbackend/binddnssec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,6 @@ void Bind2Backend::setupStatements()
d_getTSIGKeysQuery_stmt = d_dnssecdb->prepare("select name,algorithm,secret from tsigkeys", 0);
}

void Bind2Backend::release(SSqlStatement** stmt) {
delete *stmt;
*stmt = NULL;
}

void Bind2Backend::freeStatements()
{
d_getAllDomainMetadataQuery_stmt.reset();
Expand Down
2 changes: 1 addition & 1 deletion modules/gpgsqlbackend/gpgsqlbackend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void gPgSQLBackend::reconnect()

bool gPgSQLBackend::inTransaction()
{
const auto* db = dynamic_cast<SPgSQL*>(d_db);
const auto* db = dynamic_cast<SPgSQL*>(d_db.get());
if (db) {
return db->in_trx();
}
Expand Down
23 changes: 9 additions & 14 deletions modules/pipebackend/pipebackend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ static const char *kBackendId = "[PIPEBackend]";

CoWrapper::CoWrapper(const string &command, int timeout, int abiVersion)
{
d_cp=0;
d_command=command;
d_timeout=timeout;
d_abiVersion = abiVersion;
Expand All @@ -56,11 +55,8 @@ CoWrapper::CoWrapper(const string &command, int timeout, int abiVersion)

CoWrapper::~CoWrapper()
{
if(d_cp)
delete d_cp;
}


void CoWrapper::launch()
{
if(d_cp)
Expand All @@ -70,12 +66,12 @@ void CoWrapper::launch()
throw ArgException("pipe-command is not specified");

if(isUnixSocket(d_command)) {
d_cp = new UnixRemote(d_command, d_timeout);
d_cp = std::unique_ptr<CoRemote>(new UnixRemote(d_command, d_timeout));
}
else {
auto coprocess = new CoProcess(d_command, d_timeout);
auto coprocess = std::unique_ptr<CoProcess>(new CoProcess(d_command, d_timeout));
coprocess->launch();
d_cp = coprocess;
d_cp = std::move(coprocess);
}

d_cp->send("HELO\t"+std::to_string(d_abiVersion));
Expand All @@ -92,8 +88,7 @@ void CoWrapper::send(const string &line)
return;
}
catch(PDNSException &ae) {
delete d_cp;
d_cp=0;
d_cp.reset();
throw;
}
}
Expand All @@ -106,16 +101,14 @@ void CoWrapper::receive(string &line)
}
catch(PDNSException &ae) {
g_log<<Logger::Warning<<kBackendId<<" Unable to receive data from coprocess. "<<ae.reason<<endl;
delete d_cp;
d_cp=0;
d_cp.reset();
throw;
}
}

PipeBackend::PipeBackend(const string &suffix)
{
d_disavow=false;
d_regex=nullptr;
signal(SIGCHLD, SIG_IGN);
setArgPrefix("pipe"+suffix);
try {
Expand All @@ -136,7 +129,9 @@ void PipeBackend::launch()
return;

try {
d_regex=getArg("regex").empty() ? 0 : new Regex(getArg("regex"));
if (!getArg("regex").empty()) {
d_regex = std::unique_ptr<Regex>(new Regex(getArg("regex")));
}
d_regexstr=getArg("regex");
d_abiVersion = getArgAsNum("abi-version");
d_coproc=unique_ptr<CoWrapper> (new CoWrapper(getArg("command"), getArgAsNum("timeout"), getArgAsNum("abi-version")));
Expand All @@ -154,7 +149,7 @@ void PipeBackend::launch()
void PipeBackend::cleanup()
{
d_coproc.reset(0);
delete d_regex;
d_regex.reset();
d_regexstr = string();
d_abiVersion = 0;
}
Expand Down
6 changes: 3 additions & 3 deletions modules/pipebackend/pipebackend.hh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public:
void send(const string &line);
void receive(string &line);
private:
CoRemote* d_cp;
std::unique_ptr<CoRemote> d_cp;
string d_command;
void launch();
int d_timeout;
Expand All @@ -62,10 +62,10 @@ public:
private:
void launch();
void cleanup();
unique_ptr<CoWrapper> d_coproc;
std::unique_ptr<CoWrapper> d_coproc;
std::unique_ptr<Regex> d_regex;
DNSName d_qname;
QType d_qtype;
Regex* d_regex;
string d_regexstr;
bool d_disavow;
int d_abiVersion;
Expand Down
6 changes: 1 addition & 5 deletions pdns/auth-carbon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

#include "namespaces.hh"

void* carbonDumpThread(void*)
void carbonDumpThread()
try
{
setThreadName("pdns/carbonDump");
Expand Down Expand Up @@ -85,20 +85,16 @@ try
}
sleep(arg().asNum("carbon-interval"));
}
return 0;
}
catch(std::exception& e)
{
g_log<<Logger::Error<<"Carbon thread died: "<<e.what()<<endl;
return 0;
}
catch(PDNSException& e)
{
g_log<<Logger::Error<<"Carbon thread died, PDNSException: "<<e.reason<<endl;
return 0;
}
catch(...)
{
g_log<<Logger::Error<<"Carbon thread died"<<endl;
return 0;
}
36 changes: 18 additions & 18 deletions pdns/auth-packetcache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,44 +63,44 @@ AuthPacketCache::~AuthPacketCache()
}
}

bool AuthPacketCache::get(DNSPacket *p, DNSPacket *cached)
bool AuthPacketCache::get(DNSPacket& p, DNSPacket& cached)
{
if(!d_ttl) {
return false;
}

cleanupIfNeeded();

uint32_t hash = canHashPacket(p->getString());
p->setHash(hash);
uint32_t hash = canHashPacket(p.getString());
p.setHash(hash);

string value;
bool haveSomething;
time_t now = time(nullptr);
auto& mc = getMap(p->qdomain);
auto& mc = getMap(p.qdomain);
{
TryReadLock rl(&mc.d_mut);
if(!rl.gotIt()) {
S.inc("deferred-packetcache-lookup");
return false;
}

haveSomething = getEntryLocked(mc.d_map, p->getString(), hash, p->qdomain, p->qtype.getCode(), p->d_tcp, now, value);
haveSomething = getEntryLocked(mc.d_map, p.getString(), hash, p.qdomain, p.qtype.getCode(), p.d_tcp, now, value);
}

if (!haveSomething) {
(*d_statnummiss)++;
return false;
}

if(cached->noparse(value.c_str(), value.size()) < 0) {
if(cached.noparse(value.c_str(), value.size()) < 0) {
return false;
}

(*d_statnumhit)++;
cached->spoofQuestion(p); // for correct case
cached->qdomain = p->qdomain;
cached->qtype = p->qtype;
cached.spoofQuestion(p); // for correct case
cached.qdomain = p.qdomain;
cached.qtype = p.qtype;

return true;
}
Expand All @@ -110,37 +110,37 @@ bool AuthPacketCache::entryMatches(cmap_t::index<HashTag>::type::iterator& iter,
return iter->tcp == tcp && iter->qtype == qtype && iter->qname == qname && queryMatches(iter->query, query, qname);
}

void AuthPacketCache::insert(DNSPacket *q, DNSPacket *r, unsigned int maxTTL)
void AuthPacketCache::insert(DNSPacket& q, DNSPacket& r, unsigned int maxTTL)
{
if(!d_ttl) {
return;
}

cleanupIfNeeded();

if (ntohs(q->d.qdcount) != 1) {
if (ntohs(q.d.qdcount) != 1) {
return; // do not try to cache packets with multiple questions
}

if (q->qclass != QClass::IN) // we only cache the INternet
if (q.qclass != QClass::IN) // we only cache the INternet
return;

uint32_t ourttl = std::min(d_ttl, maxTTL);
if (ourttl == 0) {
return;
}

uint32_t hash = q->getHash();
uint32_t hash = q.getHash();
time_t now = time(nullptr);
CacheEntry entry;
entry.hash = hash;
entry.created = now;
entry.ttd = now + ourttl;
entry.qname = q->qdomain;
entry.qtype = q->qtype.getCode();
entry.value = r->getString();
entry.tcp = r->d_tcp;
entry.query = q->getString();
entry.qname = q.qdomain;
entry.qtype = q.qtype.getCode();
entry.value = r.getString();
entry.tcp = r.d_tcp;
entry.query = q.getString();

auto& mc = getMap(entry.qname);
{
Expand Down
4 changes: 2 additions & 2 deletions pdns/auth-packetcache.hh
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ public:
AuthPacketCache(size_t mapsCount=1024);
~AuthPacketCache();

void insert(DNSPacket *q, DNSPacket *r, uint32_t maxTTL); //!< We copy the contents of *p into our cache. Do not needlessly call this to insert questions already in the cache as it wastes resources
void insert(DNSPacket& q, DNSPacket& r, uint32_t maxTTL); //!< We copy the contents of *p into our cache. Do not needlessly call this to insert questions already in the cache as it wastes resources

bool get(DNSPacket *p, DNSPacket *q); //!< We return a dynamically allocated copy out of our cache. You need to delete it. You also need to spoof in the right ID with the DNSPacket.spoofID() method.
bool get(DNSPacket& p, DNSPacket& q); //!< You need to spoof in the right ID with the DNSPacket.spoofID() method.

void cleanup(); //!< force the cache to preen itself from expired packets
uint64_t purge();
Expand Down
8 changes: 3 additions & 5 deletions pdns/backends/gsql/gsqlbackend.hh
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,13 @@ public:
virtual ~GSQLBackend()
{
freeStatements();
if(d_db)
delete d_db;
d_db.reset();
}

void setDB(SSql *db)
{
freeStatements();
delete d_db;
d_db=db;
d_db=std::unique_ptr<SSql>(db);
if (d_db) {
d_db->setLog(::arg().mustDo("query-logging"));
allocateStatements();
Expand Down Expand Up @@ -399,7 +397,7 @@ private:
unique_ptr<SSqlStatement> d_SearchCommentsQuery_stmt;

protected:
SSql *d_db{nullptr};
std::unique_ptr<SSql> d_db{nullptr};
bool d_dnssecQueries;
bool d_inTransaction{false};
};
Expand Down
14 changes: 7 additions & 7 deletions pdns/calidns.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ static po::variables_map g_vm;

static bool g_quiet;

static void* recvThread(const vector<Socket*>* sockets)
static void* recvThread(const vector<std::unique_ptr<Socket>>* sockets)
{
vector<pollfd> rfds, fds;
for(const auto& s : *sockets) {
Expand Down Expand Up @@ -170,7 +170,7 @@ static void replaceEDNSClientSubnet(vector<uint8_t>* packet, const Netmask& ecsR
memcpy(&packet->at(packetSize - sizeof(addr)), &addr, sizeof(addr));
}

static void sendPackets(const vector<Socket*>* sockets, const vector<vector<uint8_t>* >& packets, int qps, ComboAddress dest, const Netmask& ecsRange)
static void sendPackets(const vector<std::unique_ptr<Socket>>& sockets, const vector<vector<uint8_t>* >& packets, int qps, ComboAddress dest, const Netmask& ecsRange)
{
unsigned int burst=100;
const auto nsecPerBurst=1*(unsigned long)(burst*1000000000.0/qps);
Expand Down Expand Up @@ -200,7 +200,7 @@ static void sendPackets(const vector<Socket*>* sockets, const vector<vector<uint
}

fillMSGHdr(&u.msgh, &u.iov, nullptr, 0, (char*)&(*p)[0], p->size(), &dest);
if((ret=sendmsg((*sockets)[count % sockets->size()]->getHandle(),
if((ret=sendmsg(sockets[count % sockets.size()]->getHandle(),
&u.msgh, 0)))
if(ret < 0)
unixDie("sendmsg");
Expand Down Expand Up @@ -413,7 +413,7 @@ try
cout<<"Generated "<<unknown.size()<<" ready to use queries"<<endl;
}

vector<Socket*> sockets;
vector<std::unique_ptr<Socket>> sockets;
ComboAddress dest;
try {
dest = ComboAddress(g_vm["destination"].as<string>(), 53);
Expand All @@ -423,11 +423,11 @@ try
return EXIT_FAILURE;
}
for(int i=0; i < 24; ++i) {
Socket *sock = new Socket(dest.sin4.sin_family, SOCK_DGRAM);
auto sock = make_unique<Socket>(dest.sin4.sin_family, SOCK_DGRAM);
// sock->connect(dest);
setSocketSendBuffer(sock->getHandle(), 2000000);
setSocketReceiveBuffer(sock->getHandle(), 2000000);
sockets.push_back(sock);
sockets.push_back(std::move(sock));
}
new thread(recvThread, &sockets);
uint32_t qps;
Expand Down Expand Up @@ -479,7 +479,7 @@ try
DTime dt;
dt.set();

sendPackets(&sockets, toSend, qps, dest, ecsRange);
sendPackets(sockets, toSend, qps, dest, ecsRange);

const auto udiff = dt.udiffNoReset();
const auto realqps=toSend.size()/(udiff/1000000.0);
Expand Down
Loading

0 comments on commit c2826d2

Please sign in to comment.