From 50945756310b78734342fb20f3e25848ba7e2eeb Mon Sep 17 00:00:00 2001 From: Sergiu Deitsch Date: Thu, 4 Jan 2024 23:47:06 +0100 Subject: [PATCH] fix: reduce manual resource management --- src/glog/logging.h | 9 +-- src/googletest.h | 83 ++++++++++-------------- src/logging.cc | 48 +++++++------- src/logging_unittest.cc | 42 ++++++------ src/raw_logging.cc | 2 +- src/symbolize.cc | 20 +----- src/utilities.cc | 14 ++-- src/utilities.h | 137 +++++++++++++++++++++++++++++++++------- src/vlog_is_on.cc | 10 +-- src/windows/dirent.h | 18 ++---- 10 files changed, 219 insertions(+), 164 deletions(-) diff --git a/src/glog/logging.h b/src/glog/logging.h index 757824315..8fd58ee19 100644 --- a/src/glog/logging.h +++ b/src/glog/logging.h @@ -965,12 +965,13 @@ namespace google { LOG_OCCURRENCES, &what_to_do) \ .stream() -namespace glog_internal_namespace_ { +namespace logging { +namespace internal { template struct CompileAssert {}; struct CrashReason; - -} // namespace glog_internal_namespace_ +} // namespace internal +} // namespace logging #define LOG_EVERY_N(severity, n) \ SOME_KIND_OF_LOG_EVERY_N(severity, (n), google::LogMessage::SendToLog) @@ -1334,7 +1335,7 @@ class GLOG_EXPORT LogMessage { void (LogMessage::*send_method)()); // Used to fill in crash information during LOG(FATAL) failures. - void RecordCrashReason(glog_internal_namespace_::CrashReason* reason); + void RecordCrashReason(logging::internal::CrashReason* reason); // Counts of messages sent at each priority: static int64 num_messages_[NUM_SEVERITIES]; // under log_mutex diff --git a/src/googletest.h b/src/googletest.h index 3f69a15ac..0e81de012 100644 --- a/src/googletest.h +++ b/src/googletest.h @@ -1,4 +1,4 @@ -// Copyright (c) 2009, Google Inc. +// Copyright (c) 2024, Google Inc. // All rights reserved. // // Redistribution and use in source and binary forms, with or without @@ -73,7 +73,7 @@ extern void (*g_logging_fail_func)(); extern void GetExistingTempDirectories(std::vector& list); extern int posix_strerror_r(int err, char* buf, size_t len); extern std::string StrError(int err); -} +} // namespace google #undef GLOG_EXPORT #define GLOG_EXPORT @@ -300,60 +300,53 @@ static inline void RunSpecifiedBenchmarks() { class CapturedStream { public: CapturedStream(int fd, string filename) - : fd_(fd), - - filename_(std::move(filename)) { + : fd_(fd), filename_(std::move(filename)) { Capture(); } - ~CapturedStream() { - if (uncaptured_fd_ != -1) { - CHECK(close(uncaptured_fd_) != -1); - } - } - // Start redirecting output to a file void Capture() { // Keep original stream for later - CHECK(uncaptured_fd_ == -1) << ", Stream " << fd_ << " already captured!"; - uncaptured_fd_ = dup(fd_); - CHECK(uncaptured_fd_ != -1); + CHECK(!uncaptured_fd_) << ", Stream " << fd_ << " already captured!"; + uncaptured_fd_.reset(dup(fd_)); + CHECK(uncaptured_fd_); // Open file to save stream to - int cap_fd = open(filename_.c_str(), O_CREAT | O_TRUNC | O_WRONLY, - S_IRUSR | S_IWUSR); - CHECK(cap_fd != -1); + FileDescriptor cap_fd{open(filename_.c_str(), O_CREAT | O_TRUNC | O_WRONLY, + S_IRUSR | S_IWUSR)}; + CHECK(cap_fd); // Send stdout/stderr to this file fflush(nullptr); - CHECK(dup2(cap_fd, fd_) != -1); - CHECK(close(cap_fd) != -1); + CHECK(dup2(cap_fd.get(), fd_) != -1); + CHECK(cap_fd.close() != -1); } // Remove output redirection void StopCapture() { // Restore original stream - if (uncaptured_fd_ != -1) { + if (uncaptured_fd_) { fflush(nullptr); - CHECK(dup2(uncaptured_fd_, fd_) != -1); + CHECK(dup2(uncaptured_fd_.get(), fd_) != -1); } } const string& filename() const { return filename_; } private: - int fd_; // file descriptor being captured - int uncaptured_fd_{-1}; // where the stream was originally being sent to - string filename_; // file where stream is being saved + int fd_; // file descriptor being captured + FileDescriptor + uncaptured_fd_; // where the stream was originally being sent to + string filename_; // file where stream is being saved }; -static CapturedStream* s_captured_streams[STDERR_FILENO + 1]; +static std::unique_ptr s_captured_streams[STDERR_FILENO + 1]; // Redirect a file descriptor to a file. // fd - Should be STDOUT_FILENO or STDERR_FILENO // filename - File where output should be stored static inline void CaptureTestOutput(int fd, const string& filename) { CHECK((fd == STDOUT_FILENO) || (fd == STDERR_FILENO)); CHECK(s_captured_streams[fd] == nullptr); - s_captured_streams[fd] = new CapturedStream(fd, filename); + s_captured_streams[fd] = std::make_unique(fd, filename); } static inline void CaptureTestStdout() { CaptureTestOutput(STDOUT_FILENO, FLAGS_test_tmpdir + "/captured.out"); @@ -369,7 +362,7 @@ static inline size_t GetFileSize(FILE* file) { // Read the entire content of a file as a string static inline string ReadEntireFile(FILE* file) { const size_t file_size = GetFileSize(file); - char* const buffer = new char[file_size]; + std::vector content(file_size); size_t bytes_last_read = 0; // # of bytes read in the last fread() size_t bytes_read = 0; // # of bytes read so far @@ -380,32 +373,26 @@ static inline string ReadEntireFile(FILE* file) { // pre-determined file size is reached. do { bytes_last_read = - fread(buffer + bytes_read, 1, file_size - bytes_read, file); + fread(content.data() + bytes_read, 1, file_size - bytes_read, file); bytes_read += bytes_last_read; } while (bytes_last_read > 0 && bytes_read < file_size); - const string content = string(buffer, buffer + bytes_read); - delete[] buffer; - - return content; + return std::string(content.data(), bytes_read); } // Get the captured stdout (when fd is STDOUT_FILENO) or stderr (when // fd is STDERR_FILENO) as a string static inline string GetCapturedTestOutput(int fd) { CHECK(fd == STDOUT_FILENO || fd == STDERR_FILENO); - CapturedStream* const cap = s_captured_streams[fd]; + std::unique_ptr cap = std::move(s_captured_streams[fd]); CHECK(cap) << ": did you forget CaptureTestStdout() or CaptureTestStderr()?"; // Make sure everything is flushed. cap->StopCapture(); // Read the captured file. - FILE* const file = fopen(cap->filename().c_str(), "r"); - const string content = ReadEntireFile(file); - fclose(file); - - delete cap; - s_captured_streams[fd] = nullptr; + std::unique_ptr file{fopen(cap->filename().c_str(), "r")}; + const string content = ReadEntireFile(file.get()); + file.reset(); return content; } @@ -479,11 +466,11 @@ static inline void StringReplace(string* str, const string& oldsub, } static inline string Munge(const string& filename) { - FILE* fp = fopen(filename.c_str(), "rb"); + std::unique_ptr fp{fopen(filename.c_str(), "rb")}; CHECK(fp != nullptr) << filename << ": couldn't open"; char buf[4096]; string result; - while (fgets(buf, 4095, fp)) { + while (fgets(buf, 4095, fp.get())) { string line = MungeLine(buf); const size_t str_size = 256; char null_str[str_size]; @@ -502,19 +489,17 @@ static inline string Munge(const string& filename) { StringReplace(&line, "__ENOEXEC__", StrError(ENOEXEC)); result += line + "\n"; } - fclose(fp); return result; } static inline void WriteToFile(const string& body, const string& file) { - FILE* fp = fopen(file.c_str(), "wb"); - fwrite(body.data(), 1, body.size(), fp); - fclose(fp); + std::unique_ptr fp{fopen(file.c_str(), "wb")}; + fwrite(body.data(), 1, body.size(), fp.get()); } static inline bool MungeAndDiffTest(const string& golden_filename, CapturedStream* cap) { - if (cap == s_captured_streams[STDOUT_FILENO]) { + if (cap == s_captured_streams[STDOUT_FILENO].get()) { CHECK(cap) << ": did you forget CaptureTestStdout()?"; } else { CHECK(cap) << ": did you forget CaptureTestStderr()?"; @@ -549,11 +534,13 @@ static inline bool MungeAndDiffTest(const string& golden_filename, } static inline bool MungeAndDiffTestStderr(const string& golden_filename) { - return MungeAndDiffTest(golden_filename, s_captured_streams[STDERR_FILENO]); + return MungeAndDiffTest(golden_filename, + s_captured_streams[STDERR_FILENO].get()); } static inline bool MungeAndDiffTestStdout(const string& golden_filename) { - return MungeAndDiffTest(golden_filename, s_captured_streams[STDOUT_FILENO]); + return MungeAndDiffTest(golden_filename, + s_captured_streams[STDOUT_FILENO].get()); } // Save flags used from logging_unittest.cc. diff --git a/src/logging.cc b/src/logging.cc index 14385eb0a..ac0946134 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -1011,11 +1011,12 @@ bool LogFileObject::CreateLogfile(const string& time_pid_string) { // demand that the file is unique for our timestamp (fail if it exists). flags = flags | O_EXCL; } - int fd = open(filename, flags, static_cast(FLAGS_logfile_mode)); - if (fd == -1) return false; + FileDescriptor fd{ + open(filename, flags, static_cast(FLAGS_logfile_mode))}; + if (!fd) return false; #ifdef HAVE_FCNTL // Mark the file close-on-exec. We don't really care if this fails - fcntl(fd, F_SETFD, FD_CLOEXEC); + fcntl(fd.get(), F_SETFD, FD_CLOEXEC); // Mark the file as exclusive write access to avoid two clients logging to the // same file. This applies particularly when !FLAGS_timestamp_in_logfile_name @@ -1034,17 +1035,15 @@ bool LogFileObject::CreateLogfile(const string& time_pid_string) { w_lock.l_whence = SEEK_SET; w_lock.l_len = 0; - int wlock_ret = fcntl(fd, F_SETLK, &w_lock); + int wlock_ret = fcntl(fd.get(), F_SETLK, &w_lock); if (wlock_ret == -1) { - close(fd); // as we are failing already, do not check errors here return false; } #endif // fdopen in append mode so if the file exists it will fseek to the end - file_.reset(fdopen(fd, "a")); // Make a FILE*. - if (file_ == nullptr) { // Man, we're screwed! - close(fd); + file_.reset(fdopen(fd.release(), "a")); // Make a FILE*. + if (file_ == nullptr) { // Man, we're screwed! if (FLAGS_timestamp_in_logfile_name) { unlink(filename); // Erase the half-baked evidence: an unusable log file, // only if we just created it. @@ -1506,7 +1505,7 @@ bool LogCleaner::IsLogLastModifiedOver( // for exclusive use by the first thread, and one for shared use by // all other threads. static std::mutex fatal_msg_lock; -static CrashReason crash_reason; +static logging::internal::CrashReason crash_reason; static bool fatal_msg_exclusive = true; static LogMessage::LogMessageData fatal_msg_data_exclusive; static LogMessage::LogMessageData fatal_msg_data_shared; @@ -1861,8 +1860,7 @@ void LogMessage::SendToLog() EXCLUSIVE_LOCKS_REQUIRED(log_mutex) { } } -void LogMessage::RecordCrashReason( - glog_internal_namespace_::CrashReason* reason) { +void LogMessage::RecordCrashReason(logging::internal::CrashReason* reason) { reason->filename = fatal_msg_data_exclusive.fullname_; reason->line_number = fatal_msg_data_exclusive.line_; reason->message = fatal_msg_data_exclusive.message_text_ + @@ -2396,8 +2394,8 @@ void TruncateLogFile(const char* path, uint64 limit, uint64 keep) { if (strncmp(procfd_prefix, path, strlen(procfd_prefix))) flags |= O_NOFOLLOW; # endif - int fd = open(path, flags); - if (fd == -1) { + FileDescriptor fd{open(path, flags)}; + if (!fd) { if (errno == EFBIG) { // The log file in question has got too big for us to open. The // real fix for this would be to compile logging.cc (or probably @@ -2405,7 +2403,7 @@ void TruncateLogFile(const char* path, uint64 limit, uint64 keep) { // rather scary. // Instead just truncate the file to something we can manage # ifdef HAVE__CHSIZE_S - if (_chsize_s(fd, 0) != 0) { + if (_chsize_s(fd.get(), 0) != 0) { # else if (truncate(path, 0) == -1) { # endif @@ -2419,16 +2417,16 @@ void TruncateLogFile(const char* path, uint64 limit, uint64 keep) { return; } - if (fstat(fd, &statbuf) == -1) { + if (fstat(fd.get(), &statbuf) == -1) { PLOG(ERROR) << "Unable to fstat()"; - goto out_close_fd; + return; } // See if the path refers to a regular file bigger than the // specified limit - if (!S_ISREG(statbuf.st_mode)) goto out_close_fd; - if (statbuf.st_size <= static_cast(limit)) goto out_close_fd; - if (statbuf.st_size <= static_cast(keep)) goto out_close_fd; + if (!S_ISREG(statbuf.st_mode)) return; + if (statbuf.st_size <= static_cast(limit)) return; + if (statbuf.st_size <= static_cast(keep)) return; // This log file is too large - we need to truncate it LOG(INFO) << "Truncating " << path << " to " << keep << " bytes"; @@ -2437,8 +2435,10 @@ void TruncateLogFile(const char* path, uint64 limit, uint64 keep) { read_offset = statbuf.st_size - static_cast(keep); write_offset = 0; ssize_t bytesin, bytesout; - while ((bytesin = pread(fd, copybuf, sizeof(copybuf), read_offset)) > 0) { - bytesout = pwrite(fd, copybuf, static_cast(bytesin), write_offset); + while ((bytesin = pread(fd.get(), copybuf, sizeof(copybuf), read_offset)) > + 0) { + bytesout = + pwrite(fd.get(), copybuf, static_cast(bytesin), write_offset); if (bytesout == -1) { PLOG(ERROR) << "Unable to write to " << path; break; @@ -2454,15 +2454,13 @@ void TruncateLogFile(const char* path, uint64 limit, uint64 keep) { // end of the file after our last read() above, we lose their latest // data. Too bad ... # ifdef HAVE__CHSIZE_S - if (_chsize_s(fd, write_offset) != 0) { + if (_chsize_s(fd.get(), write_offset) != 0) { # else - if (ftruncate(fd, write_offset) == -1) { + if (ftruncate(fd.get(), write_offset) == -1) { # endif PLOG(ERROR) << "Unable to truncate " << path; } -out_close_fd: - close(fd); #else LOG(ERROR) << "No log truncation support."; #endif diff --git a/src/logging_unittest.cc b/src/logging_unittest.cc index 74d6f4110..44c1c54de 100644 --- a/src/logging_unittest.cc +++ b/src/logging_unittest.cc @@ -31,19 +31,6 @@ #include -#include "config.h" -#include "utilities.h" -#ifdef HAVE_GLOB_H -# include -#endif -#include -#ifdef HAVE_UNISTD_H -# include -#endif -#ifdef HAVE_SYS_WAIT_H -# include -#endif - #include #include #include @@ -58,10 +45,23 @@ #include #include +#include "config.h" +#ifdef HAVE_GLOB_H +# include +#endif +#include +#ifdef HAVE_UNISTD_H +# include +#endif +#ifdef HAVE_SYS_WAIT_H +# include +#endif + #include "base/commandlineflags.h" #include "glog/logging.h" #include "glog/raw_logging.h" #include "googletest.h" +#include "utilities.h" #ifdef GLOG_USE_GFLAGS # include @@ -785,19 +785,17 @@ static void CheckFile(const string& name, const string& expected_string, GetFiles(name + "*", &files); CHECK_EQ(files.size(), 1UL); - FILE* file = fopen(files[0].c_str(), "r"); + std::unique_ptr file{fopen(files[0].c_str(), "r")}; CHECK(file != nullptr) << ": could not open " << files[0]; char buf[1000]; - while (fgets(buf, sizeof(buf), file) != nullptr) { + while (fgets(buf, sizeof(buf), file.get()) != nullptr) { char* first = strstr(buf, expected_string.c_str()); // if first == nullptr, not found. // Terser than if (checkInFileOrNot && first != nullptr || !check... if (checkInFileOrNot != (first == nullptr)) { - fclose(file); return; } } - fclose(file); LOG(FATAL) << "Did " << (checkInFileOrNot ? "not " : "") << "find " << expected_string << " in " << files[0]; } @@ -1155,13 +1153,11 @@ static void TestLogPeriodically() { } namespace google { -namespace glog_internal_namespace_ { -extern // in logging.cc - bool - SafeFNMatch_(const char* pattern, size_t patt_len, const char* str, - size_t str_len); +inline namespace glog_internal_namespace_ { +// in logging.cc +extern bool SafeFNMatch_(const char* pattern, size_t patt_len, const char* str, + size_t str_len); } // namespace glog_internal_namespace_ -using glog_internal_namespace_::SafeFNMatch_; } // namespace google static bool WrapSafeFNMatch(string pattern, string str) { diff --git a/src/raw_logging.cc b/src/raw_logging.cc index 75bf0541d..388cca00f 100644 --- a/src/raw_logging.cc +++ b/src/raw_logging.cc @@ -125,7 +125,7 @@ inline static bool VADoRawLog(char** buf, size_t* size, const char* format, static const int kLogBufSize = 3000; static std::once_flag crashed; -static CrashReason crash_reason; +static logging::internal::CrashReason crash_reason; static char crash_buf[kLogBufSize + 1] = {0}; // Will end in '\0' namespace { diff --git a/src/symbolize.cc b/src/symbolize.cc index 4ef29267b..b5ce8ff37 100644 --- a/src/symbolize.cc +++ b/src/symbolize.cc @@ -376,22 +376,6 @@ static bool GetSymbolFromObjectFile(const int fd, uint64_t pc, char* out, } namespace { -// Thin wrapper around a file descriptor so that the file descriptor -// gets closed for sure. -struct FileDescriptor { - const int fd_; - explicit FileDescriptor(int fd) : fd_(fd) {} - ~FileDescriptor() { - if (fd_ >= 0) { - close(fd_); - } - } - int get() { return fd_; } - - private: - FileDescriptor(const FileDescriptor&) = delete; - void operator=(const FileDescriptor&) = delete; -}; // Helper class for reading lines from file. // @@ -520,14 +504,14 @@ static ATTRIBUTE_NOINLINE int OpenObjectFileContainingPcAndGetStartAddress( int maps_fd; NO_INTR(maps_fd = open("/proc/self/maps", O_RDONLY)); FileDescriptor wrapped_maps_fd(maps_fd); - if (wrapped_maps_fd.get() < 0) { + if (!wrapped_maps_fd) { return -1; } int mem_fd; NO_INTR(mem_fd = open("/proc/self/mem", O_RDONLY)); FileDescriptor wrapped_mem_fd(mem_fd); - if (wrapped_mem_fd.get() < 0) { + if (!wrapped_mem_fd) { return -1; } diff --git a/src/utilities.cc b/src/utilities.cc index 4b1f5e0ba..51ddc9c9c 100644 --- a/src/utilities.cc +++ b/src/utilities.cc @@ -71,6 +71,12 @@ bool IsGoogleLoggingInitialized() { return g_program_invocation_short_name != nullptr; } +inline namespace glog_internal_namespace_ { + +constexpr int FileDescriptor::InvalidHandle; + +} // namespace glog_internal_namespace_ + } // namespace google // The following APIs are all internal. @@ -181,7 +187,7 @@ DumpStackTraceAndExit() { namespace google { -namespace glog_internal_namespace_ { +inline namespace glog_internal_namespace_ { const char* ProgramInvocationShortName() { if (g_program_invocation_short_name != nullptr) { @@ -252,10 +258,10 @@ void DumpStackTraceToString(string* stacktrace) { // We use an atomic operation to prevent problems with calling CrashReason // from inside the Mutex implementation (potentially through RAW_CHECK). -static std::atomic g_reason{nullptr}; +static std::atomic g_reason{nullptr}; -void SetCrashReason(const CrashReason* r) { - const CrashReason* expected = nullptr; +void SetCrashReason(const logging::internal::CrashReason* r) { + const logging::internal::CrashReason* expected = nullptr; g_reason.compare_exchange_strong(expected, r); } diff --git a/src/utilities.h b/src/utilities.h index 0af30234d..70de62dcc 100644 --- a/src/utilities.h +++ b/src/utilities.h @@ -35,6 +35,13 @@ #ifndef UTILITIES_H__ #define UTILITIES_H__ +#include +#include +#include +#include +#include +#include + // printf macros for size_t, in the style of inttypes.h #ifdef _LP64 # define __PRIS_PREFIX "z" @@ -53,12 +60,6 @@ #define PRIXS __PRIS_PREFIX "X" #define PRIoS __PRIS_PREFIX "o" -#include -#include -#include -#include -#include - #include "glog/logging.h" #if defined(GLOG_USE_WINDOWS_PORT) @@ -146,7 +147,26 @@ using ssize_t = std::ptrdiff_t; namespace google { -namespace glog_internal_namespace_ { +namespace logging { +namespace internal { + +struct CrashReason { + CrashReason() = default; + + const char* filename{nullptr}; + int line_number{0}; + const char* message{nullptr}; + + // We'll also store a bit of stack trace context at the time of crash as + // it may not be available later on. + void* stack[32]; + int depth{0}; +}; + +} // namespace internal +} // namespace logging + +inline namespace glog_internal_namespace_ { #if defined(__has_attribute) # if __has_attribute(noinline) @@ -179,20 +199,7 @@ const char* const_basename(const char* filepath); void DumpStackTraceToString(std::string* stacktrace); -struct CrashReason { - CrashReason() = default; - - const char* filename{nullptr}; - int line_number{0}; - const char* message{nullptr}; - - // We'll also store a bit of stack trace context at the time of crash as - // it may not be available later on. - void* stack[32]; - int depth{0}; -}; - -void SetCrashReason(const CrashReason* r); +void SetCrashReason(const logging::internal::CrashReason* r); void InitGoogleLoggingUtilities(const char* argv0); void ShutdownGoogleLoggingUtilities(); @@ -215,12 +222,96 @@ class ScopedExit final { Functor functor_; }; +// Thin wrapper around a file descriptor so that the file descriptor +// gets closed for sure. +class GLOG_NO_EXPORT FileDescriptor final { + static constexpr int InvalidHandle = -1; + + public: + constexpr FileDescriptor() noexcept : FileDescriptor{nullptr} {} + constexpr explicit FileDescriptor(int fd) noexcept : fd_{fd} {} + // NOLINTNEXTLINE(google-explicit-constructor) + constexpr FileDescriptor(std::nullptr_t) noexcept : fd_{InvalidHandle} {} + + FileDescriptor(const FileDescriptor& other) = delete; + FileDescriptor& operator=(const FileDescriptor& other) = delete; + + FileDescriptor(FileDescriptor&& other) noexcept : fd_{other.release()} {} + FileDescriptor& operator=(FileDescriptor&& other) noexcept { + // Close the file descriptor being held and assign a new file descriptor + // previously held by 'other' without closing it. + reset(other.release()); + return *this; + } + + constexpr explicit operator bool() const noexcept { + return fd_ != InvalidHandle; + } + + constexpr int get() const noexcept { return fd_; } + + int release() noexcept { return std::exchange(fd_, InvalidHandle); } + void reset(std::nullptr_t) noexcept { safe_close(); } + void reset() noexcept { reset(nullptr); } + void reset(int fd) noexcept { + reset(); + fd_ = fd; + } + + int close() noexcept { return unsafe_close(); } + + ~FileDescriptor() { safe_close(); } + + private: + int unsafe_close() noexcept { return ::close(release()); } + void safe_close() noexcept { + if (*this) { + unsafe_close(); + } + } + + int fd_; +}; + +// Provide variants of (in)equality comparison operators to avoid constructing +// temporaries. + +inline bool operator==(const FileDescriptor& lhs, int rhs) noexcept { + return lhs.get() == rhs; +} + +inline bool operator==(int lhs, const FileDescriptor& rhs) noexcept { + return rhs == lhs; +} + +inline bool operator!=(const FileDescriptor& lhs, int rhs) noexcept { + return !(lhs == rhs); +} + +inline bool operator!=(int lhs, const FileDescriptor& rhs) noexcept { + return !(lhs == rhs); +} + +inline bool operator==(const FileDescriptor& lhs, std::nullptr_t) noexcept { + return !lhs; +} + +inline bool operator==(std::nullptr_t, const FileDescriptor& rhs) noexcept { + return !rhs; +} + +inline bool operator!=(const FileDescriptor& lhs, std::nullptr_t) noexcept { + return static_cast(lhs); +} + +inline bool operator!=(std::nullptr_t, const FileDescriptor& rhs) noexcept { + return static_cast(rhs); +} + } // namespace glog_internal_namespace_ } // namespace google -using namespace google::glog_internal_namespace_; - template <> struct std::default_delete { void operator()(FILE* p) const noexcept { fclose(p); } diff --git a/src/vlog_is_on.cc b/src/vlog_is_on.cc index 24044259d..d6d308899 100644 --- a/src/vlog_is_on.cc +++ b/src/vlog_is_on.cc @@ -50,18 +50,14 @@ using std::string; namespace google { -namespace glog_internal_namespace_ { - -// Used by logging_unittests.cc so can't make it static here. -GLOG_EXPORT bool SafeFNMatch_(const char* pattern, size_t patt_len, - const char* str, size_t str_len); +inline namespace glog_internal_namespace_ { // Implementation of fnmatch that does not need 0-termination // of arguments and does not allocate any memory, // but we only support "*" and "?" wildcards, not the "[...]" patterns. // It's not a static function for the unittest. -GLOG_EXPORT bool SafeFNMatch_(const char* pattern, size_t patt_len, - const char* str, size_t str_len) { +GLOG_NO_EXPORT bool SafeFNMatch_(const char* pattern, size_t patt_len, + const char* str, size_t str_len) { size_t p = 0; size_t s = 0; while (true) { diff --git a/src/windows/dirent.h b/src/windows/dirent.h index 7e818ef7c..9ed3845fb 100644 --- a/src/windows/dirent.h +++ b/src/windows/dirent.h @@ -35,6 +35,7 @@ #include #include #include +#include /* Indicates that d_type field is available in dirent structure */ #define _DIRENT_HAVE_D_TYPE @@ -622,8 +623,6 @@ static WIN32_FIND_DATAW* dirent_next(_WDIR* dirp) { * Open directory stream using plain old C-string. */ static DIR* opendir(const char* dirname) { - struct DIR* dirp; - /* Must have directory name */ if (dirname == nullptr || dirname[0] == '\0') { dirent_set_errno(ENOENT); @@ -631,7 +630,9 @@ static DIR* opendir(const char* dirname) { } /* Allocate memory for DIR structure */ - dirp = (DIR*)malloc(sizeof(struct DIR)); + std::unique_ptr dirp{ + static_cast(malloc(sizeof(struct DIR))), &free}; + if (!dirp) { return nullptr; } @@ -649,23 +650,18 @@ static DIR* opendir(const char* dirname) { * the output buffer is too small to contain the resulting * string. */ - goto exit_free; + return nullptr; } /* Open directory stream using wide-character name */ dirp->wdirp = _wopendir(wname); if (!dirp->wdirp) { - goto exit_free; + return nullptr; } } /* Success */ - return dirp; - - /* Failure */ -exit_free: - free(dirp); - return nullptr; + return dirp.release(); } /*