From: Remi Gacogne Date: Wed, 10 Jun 2020 07:52:43 +0000 (+0200) Subject: Wrap more FILE objects in smart pointers X-Git-Tag: rec-4.4.0-beta1~54^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e30bc0cff78b18cba74968fed262a6d42f99a2bf;p=thirdparty%2Fpdns.git Wrap more FILE objects in smart pointers --- diff --git a/modules/pipebackend/coprocess.cc b/modules/pipebackend/coprocess.cc index 8a5183977f..52ca5f95a3 100644 --- a/modules/pipebackend/coprocess.cc +++ b/modules/pipebackend/coprocess.cc @@ -232,12 +232,7 @@ UnixRemote::UnixRemote(const string& path, int timeout) if(connect(d_fd, (struct sockaddr*)&remote, sizeof(remote)) < 0) unixDie("Unable to connect to remote '"+path+"' using UNIX domain socket"); - d_fp = fdopen(d_fd, "r"); -} - -UnixRemote::~UnixRemote() -{ - fclose(d_fp); + d_fp = std::unique_ptr(fdopen(d_fd, "r"), fclose); } void UnixRemote::send(const string& line) @@ -250,7 +245,7 @@ void UnixRemote::send(const string& line) void UnixRemote::receive(string& line) { line.clear(); - stringfgets(d_fp, line); + stringfgets(d_fp.get(), line); trim_right(line); } diff --git a/modules/pipebackend/coprocess.hh b/modules/pipebackend/coprocess.hh index 84c516e3d0..87099db566 100644 --- a/modules/pipebackend/coprocess.hh +++ b/modules/pipebackend/coprocess.hh @@ -61,12 +61,11 @@ class UnixRemote : public CoRemote { public: UnixRemote(const string &path, int timeout=0); - ~UnixRemote(); void sendReceive(const string &send, string &receive) override; void receive(string &rcv) override; void send(const string &send) override; private: int d_fd; - FILE *d_fp; + std::unique_ptr d_fp{nullptr, fclose}; }; bool isUnixSocket(const string& fname); diff --git a/pdns/dnsdist-cache.cc b/pdns/dnsdist-cache.cc index c97c674453..c31a0a554c 100644 --- a/pdns/dnsdist-cache.cc +++ b/pdns/dnsdist-cache.cc @@ -447,12 +447,12 @@ uint64_t DNSDistPacketCache::getEntriesCount() uint64_t DNSDistPacketCache::dump(int fd) { - FILE * fp = fdopen(dup(fd), "w"); + auto fp = std::unique_ptr(fdopen(dup(fd), "w"), fclose); if (fp == nullptr) { return 0; } - fprintf(fp, "; dnsdist's packet cache dump follows\n;\n"); + fprintf(fp.get(), "; dnsdist's packet cache dump follows\n;\n"); uint64_t count = 0; time_t now = time(nullptr); @@ -465,14 +465,13 @@ uint64_t DNSDistPacketCache::dump(int fd) count++; try { - fprintf(fp, "%s %" PRId64 " %s ; key %" PRIu32 ", length %" PRIu16 ", tcp %d, added %" PRId64 "\n", value.qname.toString().c_str(), static_cast(value.validity - now), QType(value.qtype).getName().c_str(), entry.first, value.len, value.tcp, static_cast(value.added)); + fprintf(fp.get(), "%s %" PRId64 " %s ; key %" PRIu32 ", length %" PRIu16 ", tcp %d, added %" PRId64 "\n", value.qname.toString().c_str(), static_cast(value.validity - now), QType(value.qtype).getName().c_str(), entry.first, value.len, value.tcp, static_cast(value.added)); } catch(...) { - fprintf(fp, "; error printing '%s'\n", value.qname.empty() ? "EMPTY" : value.qname.toString().c_str()); + fprintf(fp.get(), "; error printing '%s'\n", value.qname.empty() ? "EMPTY" : value.qname.toString().c_str()); } } } - fclose(fp); return count; } diff --git a/pdns/dnspcap.cc b/pdns/dnspcap.cc index aa17af85f1..bf3ef84690 100644 --- a/pdns/dnspcap.cc +++ b/pdns/dnspcap.cc @@ -30,18 +30,20 @@ #include "namespaces.hh" PcapPacketReader::PcapPacketReader(const string& fname) : d_fname(fname) { - d_fp=fopen(fname.c_str(),"r"); - if(!d_fp) + d_fp = std::unique_ptr(fopen(fname.c_str(), "r"), fclose); + if (!d_fp) { unixDie("Unable to open file " + fname); - - int flags=fcntl(fileno(d_fp),F_GETFL,0); - fcntl(fileno(d_fp), F_SETFL,flags&(~O_NONBLOCK)); // bsd needs this in stdin (??) - + } + + int flags = fcntl(fileno(d_fp.get()), F_GETFL, 0); + fcntl(fileno(d_fp.get()), F_SETFL, flags & (~O_NONBLOCK)); // bsd needs this in stdin (??) + checkedFread(&d_pfh); - - if(d_pfh.magic != 2712847316UL) + + if (d_pfh.magic != 2712847316UL) { throw runtime_error((format("PCAP file %s has bad magic %x, should be %x") % fname % d_pfh.magic % 2712847316UL).str()); - + } + if( d_pfh.linktype==1) { d_skipMediaHeader=sizeof(struct ether_header); } @@ -55,22 +57,17 @@ PcapPacketReader::PcapPacketReader(const string& fname) : d_fname(fname) d_skipMediaHeader=16; } else throw runtime_error((format("Unsupported link type %d") % d_pfh.linktype).str()); - - d_runts = d_oversized = d_correctpackets = d_nonetheripudp = 0; -} -PcapPacketReader::~PcapPacketReader() -{ - fclose(d_fp); + d_runts = d_oversized = d_correctpackets = d_nonetheripudp = 0; } - void PcapPacketReader::checkedFreadSize(void* ptr, size_t size) { - int ret=fread(ptr, 1, size, d_fp); - if(ret < 0) + int ret = fread(ptr, 1, size, d_fp.get()); + if (ret < 0) { unixDie( (format("Error reading %d bytes from %s") % size % d_fname).str()); - + } + if(!ret) throw EofException(); @@ -231,12 +228,14 @@ PcapPacketWriter::PcapPacketWriter(const string& fname, const PcapPacketReader& PcapPacketWriter::PcapPacketWriter(const string& fname) : d_fname(fname) { - d_fp=fopen(fname.c_str(),"w"); - if(!d_fp) + d_fp = std::unique_ptr(fopen(fname.c_str(),"w"), fclose); + + if (!d_fp) { unixDie("Unable to open file"); - - int flags=fcntl(fileno(d_fp),F_GETFL,0); - fcntl(fileno(d_fp), F_SETFL,flags&(~O_NONBLOCK)); // bsd needs this in stdin (??) + } + + int flags = fcntl(fileno(d_fp.get()), F_GETFL, 0); + fcntl(fileno(d_fp.get()), F_SETFL,flags & (~O_NONBLOCK)); // bsd needs this in stdin (??) } void PcapPacketWriter::write() @@ -246,14 +245,9 @@ void PcapPacketWriter::write() } if(d_first) { - fwrite(&d_ppr->d_pfh, 1, sizeof(d_ppr->d_pfh), d_fp); + fwrite(&d_ppr->d_pfh, 1, sizeof(d_ppr->d_pfh), d_fp.get()); d_first=false; } - fwrite(&d_ppr->d_pheader, 1, sizeof(d_ppr->d_pheader), d_fp); - fwrite(d_ppr->d_buffer, 1, d_ppr->d_pheader.caplen, d_fp); -} - -PcapPacketWriter::~PcapPacketWriter() -{ - fclose(d_fp); + fwrite(&d_ppr->d_pheader, 1, sizeof(d_ppr->d_pheader), d_fp.get()); + fwrite(d_ppr->d_buffer, 1, d_ppr->d_pheader.caplen, d_fp.get()); } diff --git a/pdns/dnspcap.hh b/pdns/dnspcap.hh index a8a6e9a120..5cb73a325c 100644 --- a/pdns/dnspcap.hh +++ b/pdns/dnspcap.hh @@ -90,8 +90,6 @@ public: PcapPacketReader(const string& fname); - ~PcapPacketReader(); - template void checkedFread(T* ptr) { @@ -119,7 +117,7 @@ public: unsigned int d_runts, d_oversized, d_correctpackets, d_nonetheripudp; char d_buffer[32768]; private: - FILE* d_fp; + std::unique_ptr d_fp{nullptr, fclose}; string d_fname; unsigned int d_skipMediaHeader; }; @@ -132,12 +130,11 @@ public: void write(); void setPPR(const PcapPacketReader& ppr) { d_ppr = &ppr; } - ~PcapPacketWriter(); private: string d_fname; const PcapPacketReader* d_ppr{nullptr}; - FILE *d_fp; + std::unique_ptr d_fp{nullptr, fclose}; bool d_first{true}; }; diff --git a/pdns/dnspcap2protobuf.cc b/pdns/dnspcap2protobuf.cc index 22482737b0..664ba89e5e 100644 --- a/pdns/dnspcap2protobuf.cc +++ b/pdns/dnspcap2protobuf.cc @@ -64,7 +64,7 @@ try { PcapPacketReader pr(argv[1]); - FILE* fp = fopen(argv[2], "w"); + auto fp = std::unique_ptr(fopen(argv[2], "w"), fclose); if (!fp) { cerr<<"Error opening output file "< workers; workers.reserve(numworkers); - FILE* fp; - if(!g_vm.count("file")) - fp=fdopen(0, "r"); + std::unique_ptr fp{nullptr, fclose}; + if (!g_vm.count("file")) { + fp = std::unique_ptr(fdopen(0, "r"), fclose); + } else { - fp=fopen(g_vm["file"].as().c_str(), "r"); - if(!fp) + fp = std::unique_ptr(fopen(g_vm["file"].as().c_str(), "r"), fclose); + if (!fp) { unixDie("Unable to open "+g_vm["file"].as()+" for input"); + } } pair q; string line; - while(stringfgets(fp, line)) { + while(stringfgets(fp.get(), line)) { trim_right(line); q=splitField(line, ' '); g_queries.push_back(BenchQuery(q.first, DNSRecordContent::TypeToNumber(q.second))); } - fclose(fp); - + fp.reset(); + for (unsigned int n = 0; n < numworkers; ++n) { workers.push_back(std::thread(worker)); } diff --git a/pdns/ixfrutils.cc b/pdns/ixfrutils.cc index feec7c3392..9d94c82150 100644 --- a/pdns/ixfrutils.cc +++ b/pdns/ixfrutils.cc @@ -124,30 +124,31 @@ void writeZoneToDisk(const records_t& records, const DNSName& zone, const std::s DNSRecord soa; auto serial = getSerialFromRecords(records, soa); string fname=directory +"/"+std::to_string(serial); - FILE* fp=fopen((fname+".partial").c_str(), "w"); - if(!fp) + auto fp = std::unique_ptr(fopen((fname+".partial").c_str(), "w"), fclose); + if (!fp) { throw runtime_error("Unable to open file '"+fname+".partial' for writing: "+stringerror()); + } records_t soarecord; soarecord.insert(soa); - if(fprintf(fp, "$ORIGIN %s\n", zone.toString().c_str()) < 0) { + if (fprintf(fp.get(), "$ORIGIN %s\n", zone.toString().c_str()) < 0) { string error = "Error writing to zone file for " + zone.toLogString() + " in file " + fname + ".partial" + ": " + stringerror(); - fclose(fp); + fp.reset(); unlink((fname+".partial").c_str()); throw std::runtime_error(error); } try { - writeRecords(fp, soarecord); - writeRecords(fp, records); - writeRecords(fp, soarecord); + writeRecords(fp.get(), soarecord); + writeRecords(fp.get(), records); + writeRecords(fp.get(), soarecord); } catch (runtime_error &e) { - fclose(fp); + fp.reset(); unlink((fname+".partial").c_str()); throw runtime_error("Error closing zone file for " + zone.toLogString() + " in file " + fname + ".partial" + ": " + e.what()); } - if(fclose(fp) != 0) { + if (fclose(fp.release()) != 0) { string error = "Error closing zone file for " + zone.toLogString() + " in file " + fname + ".partial" + ": " + stringerror(); unlink((fname+".partial").c_str()); throw std::runtime_error(error); diff --git a/pdns/recursordist/test-negcache_cc.cc b/pdns/recursordist/test-negcache_cc.cc index 5a96f2fd0a..198c8c515a 100644 --- a/pdns/recursordist/test-negcache_cc.cc +++ b/pdns/recursordist/test-negcache_cc.cc @@ -436,19 +436,19 @@ BOOST_AUTO_TEST_CASE(test_dumpToFile) cache.add(genNegCacheEntry(DNSName("www1.powerdns.com"), DNSName("powerdns.com"), now)); cache.add(genNegCacheEntry(DNSName("www2.powerdns.com"), DNSName("powerdns.com"), now)); - FILE* fp = tmpfile(); + auto fp = std::unique_ptr(tmpfile(), fclose); if (!fp) BOOST_FAIL("Temporary file could not be opened"); - cache.dumpToFile(fp); + cache.dumpToFile(fp.get()); - rewind(fp); + rewind(fp.get()); char* line = nullptr; size_t len = 0; ssize_t read; for (const auto& str : expected) { - read = getline(&line, &len, fp); + read = getline(&line, &len, fp.get()); if (read == -1) BOOST_FAIL("Unable to read a line from the temp file"); BOOST_CHECK_EQUAL(line, str); @@ -460,8 +460,6 @@ BOOST_AUTO_TEST_CASE(test_dumpToFile) last allocation if any. */ free(line); } - - fclose(fp); } BOOST_AUTO_TEST_CASE(test_count)