From: Remi Gacogne Date: Fri, 15 Feb 2019 12:18:59 +0000 (+0100) Subject: rec: Wrap FILE* in smart pointers X-Git-Tag: auth-4.2.0-beta1~17^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5e1f23ca6486ea8ac59a55a2921d978e1c69a40c;p=pdns rec: Wrap FILE* in smart pointers --- diff --git a/pdns/dnssecinfra.cc b/pdns/dnssecinfra.cc index 186463d74..8d9f9da62 100644 --- a/pdns/dnssecinfra.cc +++ b/pdns/dnssecinfra.cc @@ -52,15 +52,16 @@ using namespace boost::assign; shared_ptr DNSCryptoKeyEngine::makeFromISCFile(DNSKEYRecordContent& drc, const char* fname) { string sline, isc; - FILE *fp=fopen(fname, "r"); + auto fp = std::unique_ptr(fopen(fname, "r"), fclose); if(!fp) { throw runtime_error("Unable to read file '"+string(fname)+"' for generating DNS Private Key"); } - while(stringfgets(fp, sline)) { + while(stringfgets(fp.get(), sline)) { isc += sline; } - fclose(fp); + fp.reset(); + shared_ptr dke = makeFromISCString(drc, isc); if(!dke->checkKey()) { throw runtime_error("Invalid DNS Private Key in file '"+string(fname)); diff --git a/pdns/misc.cc b/pdns/misc.cc index 9293a4924..10f799a74 100644 --- a/pdns/misc.cc +++ b/pdns/misc.cc @@ -866,11 +866,12 @@ bool stringfgets(FILE* fp, std::string& line) bool readFileIfThere(const char* fname, std::string* line) { line->clear(); - FILE* fp = fopen(fname, "r"); + auto fp = std::unique_ptr(fopen(fname, "r"), fclose); if(!fp) return false; - stringfgets(fp, *line); - fclose(fp); + stringfgets(fp.get(), *line); + fp.reset(); + return true; } diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc index 0259c6875..92da0a805 100644 --- a/pdns/rec_channel_rec.cc +++ b/pdns/rec_channel_rec.cc @@ -173,14 +173,13 @@ string doGetParameter(T begin, T end) static uint64_t dumpNegCache(NegCache& negcache, int fd) { - FILE* fp=fdopen(dup(fd), "w"); + auto fp = std::unique_ptr(fdopen(dup(fd), "w"), fclose); if(!fp) { // dup probably failed return 0; } uint64_t ret; - fprintf(fp, "; negcache dump from thread follows\n;\n"); - ret = negcache.dumpToFile(fp); - fclose(fp); + fprintf(fp.get(), "; negcache dump from thread follows\n;\n"); + ret = negcache.dumpToFile(fp.get()); return ret; } @@ -307,14 +306,13 @@ string doDumpRPZ(T begin, T end) return "Error opening dump file for writing: "+string(strerror(errno))+"\n"; } - FILE* fp = fdopen(fd, "w"); + auto fp = std::unique_ptr(fdopen(fd, "w"), fclose); if (!fp) { close(fd); return "Error converting file descriptor: "+string(strerror(errno))+"\n"; } - zone->dump(fp); - fclose(fp); + zone->dump(fp.get()); return "done\n"; } diff --git a/pdns/recpacketcache.cc b/pdns/recpacketcache.cc index 29262159e..e474b0d21 100644 --- a/pdns/recpacketcache.cc +++ b/pdns/recpacketcache.cc @@ -150,13 +150,6 @@ bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& } -void RecursorPacketCache::insertResponsePacket(unsigned int tag, uint32_t qhash, std::string&& query, const DNSName& qname, uint16_t qtype, uint16_t qclass, std::string&& responsePacket, time_t now, uint32_t ttl, uint16_t ecsBegin, uint16_t ecsEnd) -{ - vState valState; - boost::optional pb(boost::none); - insertResponsePacket(tag, qhash, std::move(query), qname, qtype, qclass, std::move(responsePacket), now, ttl, valState, ecsBegin, ecsEnd, std::move(pb)); -} - void RecursorPacketCache::insertResponsePacket(unsigned int tag, uint32_t qhash, std::string&& query, const DNSName& qname, uint16_t qtype, uint16_t qclass, std::string&& responsePacket, time_t now, uint32_t ttl, const vState& valState, uint16_t ecsBegin, uint16_t ecsEnd, boost::optional&& protobufMessage) { auto& idx = d_packetCache.get(); @@ -226,11 +219,11 @@ void RecursorPacketCache::doPruneTo(unsigned int maxCached) uint64_t RecursorPacketCache::doDump(int fd) { - FILE* fp=fdopen(dup(fd), "w"); + auto fp = std::unique_ptr(fdopen(dup(fd), "w"), fclose); if(!fp) { // dup probably failed return 0; } - fprintf(fp, "; main packet cache dump from thread follows\n;\n"); + fprintf(fp.get(), "; main packet cache dump from thread follows\n;\n"); const auto& sidx=d_packetCache.get<1>(); uint64_t count=0; @@ -238,13 +231,12 @@ uint64_t RecursorPacketCache::doDump(int fd) for(auto i=sidx.cbegin(); i != sidx.cend(); ++i) { count++; try { - fprintf(fp, "%s %" PRId64 " %s ; tag %d\n", i->d_name.toString().c_str(), static_cast(i->d_ttd - now), DNSRecordContent::NumberToType(i->d_type).c_str(), i->d_tag); + fprintf(fp.get(), "%s %" PRId64 " %s ; tag %d\n", i->d_name.toString().c_str(), static_cast(i->d_ttd - now), DNSRecordContent::NumberToType(i->d_type).c_str(), i->d_tag); } catch(...) { - fprintf(fp, "; error printing '%s'\n", i->d_name.empty() ? "EMPTY" : i->d_name.toString().c_str()); + fprintf(fp.get(), "; error printing '%s'\n", i->d_name.empty() ? "EMPTY" : i->d_name.toString().c_str()); } } - fclose(fp); return count; } diff --git a/pdns/recpacketcache.hh b/pdns/recpacketcache.hh index 36a76c650..395d9101c 100644 --- a/pdns/recpacketcache.hh +++ b/pdns/recpacketcache.hh @@ -56,7 +56,6 @@ public: bool getResponsePacket(unsigned int tag, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, uint32_t* qhash); bool getResponsePacket(unsigned int tag, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, uint32_t* qhash, uint16_t* ecsBegin, uint16_t* ecsEnd, RecProtoBufMessage* protobufMessage); bool getResponsePacket(unsigned int tag, const std::string& queryPacket, DNSName& qname, uint16_t* qtype, uint16_t* qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, uint32_t* qhash, uint16_t* ecsBegin, uint16_t* ecsEnd, RecProtoBufMessage* protobufMessage); - void insertResponsePacket(unsigned int tag, uint32_t qhash, std::string&& query, const DNSName& qname, uint16_t qtype, uint16_t qclass, std::string&& responsePacket, time_t now, uint32_t ttl, uint16_t ecsBegin, uint16_t ecsEnd); void insertResponsePacket(unsigned int tag, uint32_t qhash, std::string&& query, const DNSName& qname, uint16_t qtype, uint16_t qclass, std::string&& responsePacket, time_t now, uint32_t ttl, const vState& valState, uint16_t ecsBegin, uint16_t ecsEnd, boost::optional&& protobufMessage); void doPruneTo(unsigned int maxSize=250000); uint64_t doDump(int fd); diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc index 7dea10c39..f9eef316c 100644 --- a/pdns/recursor_cache.cc +++ b/pdns/recursor_cache.cc @@ -438,11 +438,11 @@ bool MemRecursorCache::updateValidationStatus(time_t now, const DNSName &qname, uint64_t MemRecursorCache::doDump(int fd) { - FILE* fp=fdopen(dup(fd), "w"); + auto fp = std::unique_ptr(fdopen(dup(fd), "w"), fclose); if(!fp) { // dup probably failed return 0; } - fprintf(fp, "; main record cache dump from thread follows\n;\n"); + fprintf(fp.get(), "; main record cache dump from thread follows\n;\n"); const auto& sidx=d_cache.get(); uint64_t count=0; @@ -451,23 +451,22 @@ uint64_t MemRecursorCache::doDump(int fd) for(const auto j : i.d_records) { count++; try { - fprintf(fp, "%s %" PRId64 " IN %s %s ; (%s) auth=%i %s\n", i.d_qname.toString().c_str(), static_cast(i.d_ttd - now), DNSRecordContent::NumberToType(i.d_qtype).c_str(), j->getZoneRepresentation().c_str(), vStates[i.d_state], i.d_auth, i.d_netmask.empty() ? "" : i.d_netmask.toString().c_str()); + fprintf(fp.get(), "%s %" PRId64 " IN %s %s ; (%s) auth=%i %s\n", i.d_qname.toString().c_str(), static_cast(i.d_ttd - now), DNSRecordContent::NumberToType(i.d_qtype).c_str(), j->getZoneRepresentation().c_str(), vStates[i.d_state], i.d_auth, i.d_netmask.empty() ? "" : i.d_netmask.toString().c_str()); } catch(...) { - fprintf(fp, "; error printing '%s'\n", i.d_qname.empty() ? "EMPTY" : i.d_qname.toString().c_str()); + fprintf(fp.get(), "; error printing '%s'\n", i.d_qname.empty() ? "EMPTY" : i.d_qname.toString().c_str()); } } for(const auto &sig : i.d_signatures) { count++; try { - fprintf(fp, "%s %" PRId64 " IN RRSIG %s ; %s\n", i.d_qname.toString().c_str(), static_cast(i.d_ttd - now), sig->getZoneRepresentation().c_str(), i.d_netmask.empty() ? "" : i.d_netmask.toString().c_str()); + fprintf(fp.get(), "%s %" PRId64 " IN RRSIG %s ; %s\n", i.d_qname.toString().c_str(), static_cast(i.d_ttd - now), sig->getZoneRepresentation().c_str(), i.d_netmask.empty() ? "" : i.d_netmask.toString().c_str()); } catch(...) { - fprintf(fp, "; error printing '%s'\n", i.d_qname.empty() ? "EMPTY" : i.d_qname.toString().c_str()); + fprintf(fp.get(), "; error printing '%s'\n", i.d_qname.empty() ? "EMPTY" : i.d_qname.toString().c_str()); } } } - fclose(fp); return count; } diff --git a/pdns/reczones.cc b/pdns/reczones.cc index e2625c4e3..45bcb814a 100644 --- a/pdns/reczones.cc +++ b/pdns/reczones.cc @@ -382,14 +382,11 @@ std::shared_ptr parseAuthAndForwards() if(!::arg()["forward-zones-file"].empty()) { g_log<(fopen(::arg()["forward-zones-file"].c_str(), "r"), fclose); + if(!fp) { throw PDNSException("Error opening forward-zones-file '"+::arg()["forward-zones-file"]+"': "+stringerror()); } - shared_ptr fp=shared_ptr(rfp, fclose); - string line; int linenum=0; uint64_t before = newMap->size(); diff --git a/pdns/rpzloader.cc b/pdns/rpzloader.cc index 8dc17311c..4180dd476 100644 --- a/pdns/rpzloader.cc +++ b/pdns/rpzloader.cc @@ -297,35 +297,33 @@ static bool dumpZoneToDisk(const DNSName& zoneName, const std::shared_ptr(fdopen(fd, "w+"), fclose); + if (!fp) { close(fd); g_log<dump(fp); + newZone->dump(fp.get()); } catch(const std::exception& e) { g_log<(fdopen(dup(fd), "w"), fclose); if (!fp) { return 0; } uint64_t count = 0; - fprintf(fp,"; edns from thread follows\n;\n"); + fprintf(fp.get(),"; edns from thread follows\n;\n"); for(const auto& eds : t_sstorage.ednsstatus) { count++; - fprintf(fp, "%s\t%d\t%s", eds.first.toString().c_str(), (int)eds.second.mode, ctime(&eds.second.modeSetAt)); + fprintf(fp.get(), "%s\t%d\t%s", eds.first.toString().c_str(), (int)eds.second.mode, ctime(&eds.second.modeSetAt)); } - fclose(fp); return count; } uint64_t SyncRes::doDumpNSSpeeds(int fd) { - FILE* fp=fdopen(dup(fd), "w"); + auto fp = std::unique_ptr(fdopen(dup(fd), "w"), fclose); if(!fp) return 0; - fprintf(fp, "; nsspeed dump from thread follows\n;\n"); + fprintf(fp.get(), "; nsspeed dump from thread follows\n;\n"); uint64_t count=0; for(const auto& i : t_sstorage.nsSpeeds) @@ -383,25 +382,24 @@ uint64_t SyncRes::doDumpNSSpeeds(int fd) count++; // an can appear hear in case of authoritative (hosted) zones - fprintf(fp, "%s -> ", i.first.toLogString().c_str()); + fprintf(fp.get(), "%s -> ", i.first.toLogString().c_str()); for(const auto& j : i.second.d_collection) { // typedef vector > collection_t; - fprintf(fp, "%s/%f ", j.first.toString().c_str(), j.second.peek()); + fprintf(fp.get(), "%s/%f ", j.first.toString().c_str(), j.second.peek()); } - fprintf(fp, "\n"); + fprintf(fp.get(), "\n"); } - fclose(fp); return count; } uint64_t SyncRes::doDumpThrottleMap(int fd) { - FILE* fp=fdopen(dup(fd), "w"); + auto fp = std::unique_ptr(fdopen(dup(fd), "w"), fclose); if(!fp) return 0; - fprintf(fp, "; throttle map dump follows\n"); - fprintf(fp, "; remote IP\tqname\tqtype\tcount\tttd\n"); + fprintf(fp.get(), "; throttle map dump follows\n"); + fprintf(fp.get(), "; remote IP\tqname\tqtype\tcount\tttd\n"); uint64_t count=0; const auto& throttleMap = t_sstorage.throttle.getThrottleMap(); @@ -409,9 +407,9 @@ uint64_t SyncRes::doDumpThrottleMap(int fd) { count++; // remote IP, dns name, qtype, count, ttd - fprintf(fp, "%s\t%s\t%d\t%u\t%s", i.first.get<0>().toString().c_str(), i.first.get<1>().toLogString().c_str(), i.first.get<2>(), i.second.count, ctime(&i.second.ttd)); + fprintf(fp.get(), "%s\t%s\t%d\t%u\t%s", i.first.get<0>().toString().c_str(), i.first.get<1>().toLogString().c_str(), i.first.get<2>(), i.second.count, ctime(&i.second.ttd)); } - fclose(fp); + return count; } diff --git a/pdns/test-dnsdist_cc.cc b/pdns/test-dnsdist_cc.cc index 4af4ba239..872592ead 100644 --- a/pdns/test-dnsdist_cc.cc +++ b/pdns/test-dnsdist_cc.cc @@ -1533,8 +1533,10 @@ BOOST_AUTO_TEST_CASE(test_isEDNSOptionInOpt) { bool found = locateEDNSOption(query, EDNSOptionCode::ECS, &optContentStart, &optContentLen); BOOST_CHECK_EQUAL(found, true); - BOOST_CHECK_EQUAL(optContentStart, optRDExpectedOffset + sizeof(uint16_t) /* RD len */ + /* option code */ 2 + /* option length */ 2); - BOOST_CHECK_EQUAL(optContentLen, sizeOfECSContent); + if (found == true) { + BOOST_CHECK_EQUAL(optContentStart, optRDExpectedOffset + sizeof(uint16_t) /* RD len */ + /* option code */ 2 + /* option length */ 2); + BOOST_CHECK_EQUAL(optContentLen, sizeOfECSContent); + } /* truncated packet */ query.resize(query.size() - 1); @@ -1554,8 +1556,10 @@ BOOST_AUTO_TEST_CASE(test_isEDNSOptionInOpt) { bool found = locateEDNSOption(query, EDNSOptionCode::ECS, &optContentStart, &optContentLen); BOOST_CHECK_EQUAL(found, true); - BOOST_CHECK_EQUAL(optContentStart, optRDExpectedOffset + sizeof(uint16_t) /* RD len */ + sizeOfCookieOption + /* option code */ 2 + /* option length */ 2); - BOOST_CHECK_EQUAL(optContentLen, sizeOfECSContent); + if (found == true) { + BOOST_CHECK_EQUAL(optContentStart, optRDExpectedOffset + sizeof(uint16_t) /* RD len */ + sizeOfCookieOption + /* option code */ 2 + /* option length */ 2); + BOOST_CHECK_EQUAL(optContentLen, sizeOfECSContent); + } /* truncated packet */ query.resize(query.size() - 1); diff --git a/pdns/test-recpacketcache_cc.cc b/pdns/test-recpacketcache_cc.cc index 365187108..5e856e1be 100644 --- a/pdns/test-recpacketcache_cc.cc +++ b/pdns/test-recpacketcache_cc.cc @@ -41,16 +41,16 @@ BOOST_AUTO_TEST_CASE(test_recPacketCacheSimple) { pw.commit(); string rpacket((const char*)&packet[0], packet.size()); - rpc.insertResponsePacket(tag, qhash, string(qpacket), qname, QType::A, QClass::IN, string(rpacket), time(0), ttd, 0, 0); + rpc.insertResponsePacket(tag, qhash, string(qpacket), qname, QType::A, QClass::IN, string(rpacket), time(0), ttd, Indeterminate, 0, 0, boost::none); BOOST_CHECK_EQUAL(rpc.size(), 1); rpc.doPruneTo(0); BOOST_CHECK_EQUAL(rpc.size(), 0); - rpc.insertResponsePacket(tag, qhash, string(qpacket), qname, QType::A, QClass::IN, string(rpacket), time(0), ttd, 0, 0); + rpc.insertResponsePacket(tag, qhash, string(qpacket), qname, QType::A, QClass::IN, string(rpacket), time(0), ttd, Indeterminate, 0, 0, boost::none); BOOST_CHECK_EQUAL(rpc.size(), 1); rpc.doWipePacketCache(qname); BOOST_CHECK_EQUAL(rpc.size(), 0); - rpc.insertResponsePacket(tag, qhash, string(qpacket), qname, QType::A, QClass::IN, string(rpacket), time(0), ttd, 0, 0); + rpc.insertResponsePacket(tag, qhash, string(qpacket), qname, QType::A, QClass::IN, string(rpacket), time(0), ttd, Indeterminate, 0, 0, boost::none); BOOST_CHECK_EQUAL(rpc.size(), 1); uint32_t qhash2 = 0; bool found = rpc.getResponsePacket(tag, qpacket, time(nullptr), &fpacket, &age, &qhash2); @@ -133,11 +133,11 @@ BOOST_AUTO_TEST_CASE(test_recPacketCache_Tags) { BOOST_CHECK(r1packet != r2packet); /* inserting a response for tag1 */ - rpc.insertResponsePacket(tag1, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r1packet), time(0), ttd, 0, 0); + rpc.insertResponsePacket(tag1, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r1packet), time(0), ttd, Indeterminate, 0, 0, boost::none); BOOST_CHECK_EQUAL(rpc.size(), 1); /* inserting a different response for tag2, should not override the first one */ - rpc.insertResponsePacket(tag2, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r2packet), time(0), ttd, 0, 0); + rpc.insertResponsePacket(tag2, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r2packet), time(0), ttd, Indeterminate, 0, 0, boost::none); BOOST_CHECK_EQUAL(rpc.size(), 2); /* remove all responses from the cache */ @@ -145,10 +145,10 @@ BOOST_AUTO_TEST_CASE(test_recPacketCache_Tags) { BOOST_CHECK_EQUAL(rpc.size(), 0); /* reinsert both */ - rpc.insertResponsePacket(tag1, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r1packet), time(0), ttd, 0, 0); + rpc.insertResponsePacket(tag1, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r1packet), time(0), ttd, Indeterminate, 0, 0, boost::none); BOOST_CHECK_EQUAL(rpc.size(), 1); - rpc.insertResponsePacket(tag2, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r2packet), time(0), ttd, 0, 0); + rpc.insertResponsePacket(tag2, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r2packet), time(0), ttd, Indeterminate, 0, 0, boost::none); BOOST_CHECK_EQUAL(rpc.size(), 2); /* remove the responses by qname, should remove both */ @@ -156,7 +156,7 @@ BOOST_AUTO_TEST_CASE(test_recPacketCache_Tags) { BOOST_CHECK_EQUAL(rpc.size(), 0); /* insert the response for tag1 */ - rpc.insertResponsePacket(tag1, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r1packet), time(0), ttd, 0, 0); + rpc.insertResponsePacket(tag1, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r1packet), time(0), ttd, Indeterminate, 0, 0, boost::none); BOOST_CHECK_EQUAL(rpc.size(), 1); /* we can retrieve it */ @@ -175,7 +175,7 @@ BOOST_AUTO_TEST_CASE(test_recPacketCache_Tags) { BOOST_CHECK_EQUAL(temphash, qhash); /* adding a response for the second tag */ - rpc.insertResponsePacket(tag2, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r2packet), time(0), ttd, 0, 0); + rpc.insertResponsePacket(tag2, qhash, string(qpacket), qname, QType::A, QClass::IN, string(r2packet), time(0), ttd, Indeterminate, 0, 0, boost::none); BOOST_CHECK_EQUAL(rpc.size(), 2); /* We still get the correct response for the first tag */