Skip to content

Commit

Permalink
fix: reduce boilerplate code
Browse files Browse the repository at this point in the history
  • Loading branch information
sergiud committed Jan 6, 2024
1 parent 2c735b8 commit 94c3d10
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 33 deletions.
4 changes: 2 additions & 2 deletions src/googletest.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ using std::vector;

namespace google {
extern void (*g_logging_fail_func)();
extern void GetExistingTempDirectories(vector<string>* list);
extern void GetExistingTempDirectories(std::vector<std::string>& list);
extern int posix_strerror_r(int err, char* buf, size_t len);
extern std::string StrError(int err);
}
Expand All @@ -80,7 +80,7 @@ extern std::string StrError(int err);

static inline string GetTempDir() {
vector<string> temp_directories_list;
google::GetExistingTempDirectories(&temp_directories_list);
google::GetExistingTempDirectories(temp_directories_list);

if (temp_directories_list.empty()) {
fprintf(stderr, "No temporary directory found\n");
Expand Down
56 changes: 25 additions & 31 deletions src/logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include <cstdint>
#include <iomanip>
#include <iterator>
#include <memory>
#include <mutex>
#include <shared_mutex>
#include <string>
Expand Down Expand Up @@ -414,7 +415,7 @@ class LogFileObject : public base::Logger {
string base_filename_;
string symlink_basename_;
string filename_extension_; // option users can specify (eg to add port#)
FILE* file_{nullptr};
std::unique_ptr<FILE> file_;
LogSeverity severity_;
uint32 bytes_since_flush_{0};
uint32 dropped_mem_length_{0};
Expand Down Expand Up @@ -947,10 +948,7 @@ LogFileObject::LogFileObject(LogSeverity severity, const char* base_filename)

LogFileObject::~LogFileObject() {
std::lock_guard<std::mutex> l{mutex_};
if (file_ != nullptr) {
fclose(file_);
file_ = nullptr;
}
file_ = nullptr;
}

void LogFileObject::SetBasename(const char* basename) {
Expand All @@ -959,7 +957,6 @@ void LogFileObject::SetBasename(const char* basename) {
if (base_filename_ != basename) {
// Get rid of old log file since we are changing names
if (file_ != nullptr) {
fclose(file_);
file_ = nullptr;
rollover_attempt_ = kRolloverAttemptFrequency - 1;
}
Expand All @@ -972,7 +969,6 @@ void LogFileObject::SetExtension(const char* ext) {
if (filename_extension_ != ext) {
// Get rid of old log file since we are changing names
if (file_ != nullptr) {
fclose(file_);
file_ = nullptr;
rollover_attempt_ = kRolloverAttemptFrequency - 1;
}
Expand All @@ -993,7 +989,7 @@ void LogFileObject::Flush() {
void LogFileObject::FlushUnlocked(
const std::chrono::system_clock::time_point& now) {
if (file_ != nullptr) {
fflush(file_);
fflush(file_.get());
bytes_since_flush_ = 0;
}
// Figure out when we are due for another flush.
Expand Down Expand Up @@ -1046,8 +1042,8 @@ bool LogFileObject::CreateLogfile(const string& time_pid_string) {
#endif

// fdopen in append mode so if the file exists it will fseek to the end
file_ = fdopen(fd, "a"); // Make a FILE*.
if (file_ == nullptr) { // Man, we're screwed!
file_.reset(fdopen(fd, "a")); // Make a FILE*.
if (file_ == nullptr) { // Man, we're screwed!
close(fd);
if (FLAGS_timestamp_in_logfile_name) {
unlink(filename); // Erase the half-baked evidence: an unusable log file,
Expand All @@ -1059,7 +1055,7 @@ bool LogFileObject::CreateLogfile(const string& time_pid_string) {
// https://github.com/golang/go/issues/27638 - make sure we seek to the end to
// append empirically replicated with wine over mingw build
if (!FLAGS_timestamp_in_logfile_name) {
if (fseek(file_, 0, SEEK_END) != 0) {
if (fseek(file_.get(), 0, SEEK_END) != 0) {
return false;
}
}
Expand Down Expand Up @@ -1128,7 +1124,6 @@ void LogFileObject::Write(
ScopedExit<decltype(cleanupLogs)> cleanupAtEnd{cleanupLogs};

if (file_length_ >> 20U >= MaxLogSize() || PidHasChanged()) {
if (file_ != nullptr) fclose(file_);
file_ = nullptr;
file_length_ = bytes_since_flush_ = dropped_mem_length_ = 0;
rollover_attempt_ = kRolloverAttemptFrequency - 1;
Expand Down Expand Up @@ -1246,7 +1241,7 @@ void LogFileObject::Write(
const string& file_header_string = file_header_stream.str();

const size_t header_len = file_header_string.size();
fwrite(file_header_string.data(), 1, header_len, file_);
fwrite(file_header_string.data(), 1, header_len, file_.get());
file_length_ += header_len;
bytes_since_flush_ += header_len;
}
Expand All @@ -1260,7 +1255,7 @@ void LogFileObject::Write(
// 4096 bytes. fwrite() returns 4096 for message lengths that are
// greater than 4096, thereby indicating an error.
errno = 0;
fwrite(message, 1, message_len, file_);
fwrite(message, 1, message_len, file_.get());
if (FLAGS_stop_logging_if_full_disk &&
errno == ENOSPC) { // disk full, stop writing to disk
stop_writing = true; // until the disk is
Expand Down Expand Up @@ -1295,9 +1290,9 @@ void LogFileObject::Write(
// 'posix_fadvise' introduced in API 21:
// * https://android.googlesource.com/platform/bionic/+/6880f936173081297be0dc12f687d341b86a4cfa/libc/libc.map.txt#732
# else
posix_fadvise(fileno(file_), static_cast<off_t>(dropped_mem_length_),
static_cast<off_t>(this_drop_length),
POSIX_FADV_DONTNEED);
posix_fadvise(
fileno(file_.get()), static_cast<off_t>(dropped_mem_length_),
static_cast<off_t>(this_drop_length), POSIX_FADV_DONTNEED);
# endif
dropped_mem_length_ = total_drop_length;
}
Expand Down Expand Up @@ -2293,17 +2288,17 @@ bool SendEmail(const char* dest, const char* subject, const char* body) {
return SendEmailInternal(dest, subject, body, true);
}

static void GetTempDirectories(vector<string>* list) {
list->clear();
static void GetTempDirectories(vector<string>& list) {
list.clear();
#ifdef GLOG_OS_WINDOWS
// On windows we'll try to find a directory in this order:
// C:/Documents & Settings/whomever/TEMP (or whatever GetTempPath() is)
// C:/TMP/
// C:/TEMP/
char tmp[MAX_PATH];
if (GetTempPathA(MAX_PATH, tmp)) list->push_back(tmp);
list->push_back("C:\\TMP\\");
list->push_back("C:\\TEMP\\");
if (GetTempPathA(MAX_PATH, tmp)) list.push_back(tmp);
list.push_back("C:\\TMP\\");
list.push_back("C:\\TEMP\\");
#else
// Directories, in order of preference. If we find a dir that
// exists, we stop adding other less-preferred dirs
Expand All @@ -2327,7 +2322,7 @@ static void GetTempDirectories(vector<string>* list) {
if (dstr[dstr.size() - 1] != '/') {
dstr += "/";
}
list->push_back(dstr);
list.push_back(dstr);

struct stat statbuf;
if (!stat(d, &statbuf) && S_ISDIR(statbuf.st_mode)) {
Expand All @@ -2338,12 +2333,12 @@ static void GetTempDirectories(vector<string>* list) {
#endif
}

static vector<string>* logging_directories_list;
static std::unique_ptr<std::vector<std::string>> logging_directories_list;

const vector<string>& GetLoggingDirectories() {
// Not strictly thread-safe but we're called early in InitGoogle().
if (logging_directories_list == nullptr) {
logging_directories_list = new vector<string>;
logging_directories_list = std::make_unique<std::vector<std::string>>();

if (!FLAGS_log_dir.empty()) {
// Ensure the specified path ends with a directory delimiter.
Expand All @@ -2355,7 +2350,7 @@ const vector<string>& GetLoggingDirectories() {
logging_directories_list->push_back(FLAGS_log_dir);
}
} else {
GetTempDirectories(logging_directories_list);
GetTempDirectories(*logging_directories_list);
#ifdef GLOG_OS_WINDOWS
char tmp[MAX_PATH];
if (GetWindowsDirectoryA(tmp, MAX_PATH))
Expand All @@ -2373,14 +2368,14 @@ const vector<string>& GetLoggingDirectories() {
// subset of the directories returned by GetLoggingDirectories().
// Thread-safe.
GLOG_NO_EXPORT
void GetExistingTempDirectories(vector<string>* list) {
void GetExistingTempDirectories(vector<string>& list) {
GetTempDirectories(list);
auto i_dir = list->begin();
while (i_dir != list->end()) {
auto i_dir = list.begin();
while (i_dir != list.end()) {
// zero arg to access means test for existence; no constant
// defined on windows
if (access(i_dir->c_str(), 0)) {
i_dir = list->erase(i_dir);
i_dir = list.erase(i_dir);
} else {
++i_dir;
}
Expand Down Expand Up @@ -2659,7 +2654,6 @@ void InitGoogleLogging(const char* argv0, CustomPrefixCallback prefix_callback,
void ShutdownGoogleLogging() {
glog_internal_namespace_::ShutdownGoogleLoggingUtilities();
LogDestination::DeleteLogDestinations();
delete logging_directories_list;
logging_directories_list = nullptr;
}

Expand Down
7 changes: 7 additions & 0 deletions src/utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
#define PRIXS __PRIS_PREFIX "X"
#define PRIoS __PRIS_PREFIX "o"

#include <cstdio>
#include <memory>
#include <string>
#include <thread>
#include <type_traits>
Expand Down Expand Up @@ -219,4 +221,9 @@ class ScopedExit final {

using namespace google::glog_internal_namespace_;

template <>
struct std::default_delete<std::FILE> {
void operator()(FILE* p) const noexcept { fclose(p); }
};

#endif // UTILITIES_H__

0 comments on commit 94c3d10

Please sign in to comment.