From 07a00828af8f79789abbf418dd86f34e1432d0fa Mon Sep 17 00:00:00 2001 From: Levi Tamasi Date: Tue, 24 May 2022 18:20:17 -0700 Subject: [PATCH] Fix potential ambiguities in/around port/sys_time.h (#10045) Summary: There are some time-related POSIX APIs that are not available on Windows (e.g. `localtime_r`), which we have worked around by providing our own implementations in `port/sys_time.h`. This workaround actually relies on some ambiguity: on Windows, a call to `localtime_r` calls `ROCKSDB_NAMESPACE::port::localtime_r` (which is pulled into `ROCKSDB_NAMESPACE` by a using-declaration), while on other platforms it calls the global `localtime_r`. This works fine as long as there is only one candidate function; however, it breaks down when there is more than one `localtime_r` visible in a scope. The patch fixes this by introducing `ROCKSDB_NAMESPACE::port::{TimeVal, GetTimeOfDay, LocalTimeR}` to eliminate any ambiguity. Pull Request resolved: https://github.com/facebook/rocksdb/pull/10045 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D36639372 Pulled By: ltamasi fbshipit-source-id: fc13dbfa421b7c8918111a6d9e24ce77e91a7c50 --- env/env_posix.cc | 6 +-- env/env_test.cc | 10 ++--- env/mock_env.cc | 6 +-- logging/env_logger.h | 6 +-- logging/log_buffer.cc | 4 +- logging/log_buffer.h | 2 +- logging/posix_logger.h | 6 +-- port/sys_time.h | 38 +++++++++++++------ port/win/port_win.cc | 2 +- port/win/win_logger.cc | 4 +- util/string_util.cc | 2 +- .../persistent_cache/hash_table_bench.cc | 10 +++-- 12 files changed, 57 insertions(+), 39 deletions(-) diff --git a/env/env_posix.cc b/env/env_posix.cc index 66848d11f..5698fcdd7 100644 --- a/env/env_posix.cc +++ b/env/env_posix.cc @@ -134,8 +134,8 @@ class PosixClock : public SystemClock { const char* NickName() const override { return kClassName(); } uint64_t NowMicros() override { - struct timeval tv; - gettimeofday(&tv, nullptr); + port::TimeVal tv; + port::GetTimeOfDay(&tv, nullptr); return static_cast(tv.tv_sec) * 1000000 + tv.tv_usec; } @@ -200,7 +200,7 @@ class PosixClock : public SystemClock { dummy.reserve(maxsize); dummy.resize(maxsize); char* p = &dummy[0]; - localtime_r(&seconds, &t); + port::LocalTimeR(&seconds, &t); snprintf(p, maxsize, "%04d/%02d/%02d-%02d:%02d:%02d ", t.tm_year + 1900, t.tm_mon + 1, t.tm_mday, t.tm_hour, t.tm_min, t.tm_sec); return dummy; diff --git a/env/env_test.cc b/env/env_test.cc index 441db0313..3ae6c1e8d 100644 --- a/env/env_test.cc +++ b/env/env_test.cc @@ -1604,9 +1604,9 @@ class TestLogger : public Logger { if (new_format[0] == '[') { // "[DEBUG] " - ASSERT_TRUE(n <= 56 + (512 - static_cast(sizeof(struct timeval)))); + ASSERT_TRUE(n <= 56 + (512 - static_cast(sizeof(port::TimeVal)))); } else { - ASSERT_TRUE(n <= 48 + (512 - static_cast(sizeof(struct timeval)))); + ASSERT_TRUE(n <= 48 + (512 - static_cast(sizeof(port::TimeVal)))); } va_end(backup_ap); } @@ -1674,9 +1674,9 @@ class TestLogger2 : public Logger { va_copy(backup_ap, ap); int n = vsnprintf(new_format, sizeof(new_format) - 1, format, backup_ap); // 48 bytes for extra information + bytes allocated - ASSERT_TRUE( - n <= 48 + static_cast(max_log_size_ - sizeof(struct timeval))); - ASSERT_TRUE(n > static_cast(max_log_size_ - sizeof(struct timeval))); + ASSERT_TRUE(n <= + 48 + static_cast(max_log_size_ - sizeof(port::TimeVal))); + ASSERT_TRUE(n > static_cast(max_log_size_ - sizeof(port::TimeVal))); va_end(backup_ap); } } diff --git a/env/mock_env.cc b/env/mock_env.cc index 0ab0f981f..5bdbe4f70 100644 --- a/env/mock_env.cc +++ b/env/mock_env.cc @@ -509,13 +509,13 @@ class TestMemLogger : public Logger { char* p = base; char* limit = base + bufsize; - struct timeval now_tv; - gettimeofday(&now_tv, nullptr); + port::TimeVal now_tv; + port::GetTimeOfDay(&now_tv, nullptr); const time_t seconds = now_tv.tv_sec; struct tm t; memset(&t, 0, sizeof(t)); struct tm* ret __attribute__((__unused__)); - ret = localtime_r(&seconds, &t); + ret = port::LocalTimeR(&seconds, &t); assert(ret); p += snprintf(p, limit - p, "%04d/%02d/%02d-%02d:%02d:%02d.%06d ", t.tm_year + 1900, t.tm_mon + 1, t.tm_mday, t.tm_hour, diff --git a/logging/env_logger.h b/logging/env_logger.h index e8e9f1abe..ac62c6f1a 100644 --- a/logging/env_logger.h +++ b/logging/env_logger.h @@ -100,11 +100,11 @@ class EnvLogger : public Logger { char* p = base; char* limit = base + bufsize; - struct timeval now_tv; - gettimeofday(&now_tv, nullptr); + port::TimeVal now_tv; + port::GetTimeOfDay(&now_tv, nullptr); const time_t seconds = now_tv.tv_sec; struct tm t; - localtime_r(&seconds, &t); + port::LocalTimeR(&seconds, &t); p += snprintf(p, limit - p, "%04d/%02d/%02d-%02d:%02d:%02d.%06d %llx ", t.tm_year + 1900, t.tm_mon + 1, t.tm_mday, t.tm_hour, t.tm_min, t.tm_sec, static_cast(now_tv.tv_usec), diff --git a/logging/log_buffer.cc b/logging/log_buffer.cc index 242d280b3..378fcbb52 100644 --- a/logging/log_buffer.cc +++ b/logging/log_buffer.cc @@ -27,7 +27,7 @@ void LogBuffer::AddLogToBuffer(size_t max_log_size, const char* format, char* limit = alloc_mem + max_log_size - 1; // store the time - gettimeofday(&(buffered_log->now_tv), nullptr); + port::GetTimeOfDay(&(buffered_log->now_tv), nullptr); // Print the message if (p < limit) { @@ -60,7 +60,7 @@ void LogBuffer::FlushBufferToLog() { for (BufferedLog* log : logs_) { const time_t seconds = log->now_tv.tv_sec; struct tm t; - if (localtime_r(&seconds, &t) != nullptr) { + if (port::LocalTimeR(&seconds, &t) != nullptr) { Log(log_level_, info_log_, "(Original Log Time %04d/%02d/%02d-%02d:%02d:%02d.%06d) %s", t.tm_year + 1900, t.tm_mon + 1, t.tm_mday, t.tm_hour, t.tm_min, diff --git a/logging/log_buffer.h b/logging/log_buffer.h index 285256e20..61d0be7df 100644 --- a/logging/log_buffer.h +++ b/logging/log_buffer.h @@ -35,7 +35,7 @@ class LogBuffer { private: // One log entry with its timestamp struct BufferedLog { - struct timeval now_tv; // Timestamp of the log + port::TimeVal now_tv; // Timestamp of the log char message[1]; // Beginning of log message }; diff --git a/logging/posix_logger.h b/logging/posix_logger.h index 115d42fdb..fa02dd752 100644 --- a/logging/posix_logger.h +++ b/logging/posix_logger.h @@ -103,11 +103,11 @@ class PosixLogger : public Logger { char* p = base; char* limit = base + bufsize; - struct timeval now_tv; - gettimeofday(&now_tv, nullptr); + port::TimeVal now_tv; + port::GetTimeOfDay(&now_tv, nullptr); const time_t seconds = now_tv.tv_sec; struct tm t; - localtime_r(&seconds, &t); + port::LocalTimeR(&seconds, &t); p += snprintf(p, limit - p, "%04d/%02d/%02d-%02d:%02d:%02d.%06d %llu ", t.tm_year + 1900, t.tm_mon + 1, t.tm_mday, t.tm_hour, t.tm_min, t.tm_sec, static_cast(now_tv.tv_usec), diff --git a/port/sys_time.h b/port/sys_time.h index 8a5440f95..d4dd2e07f 100644 --- a/port/sys_time.h +++ b/port/sys_time.h @@ -12,36 +12,52 @@ #pragma once -#if defined(OS_WIN) && defined(_MSC_VER) +#include "rocksdb/rocksdb_namespace.h" + +#if defined(OS_WIN) && (defined(_MSC_VER) || defined(__MINGW32__)) #include -#include "rocksdb/rocksdb_namespace.h" - namespace ROCKSDB_NAMESPACE { namespace port { -// Avoid including winsock2.h for this definition -struct timeval { +struct TimeVal { long tv_sec; long tv_usec; }; -void gettimeofday(struct timeval* tv, struct timezone* tz); +void GetTimeOfDay(TimeVal* tv, struct timezone* tz); -inline struct tm* localtime_r(const time_t* timep, struct tm* result) { +inline struct tm* LocalTimeR(const time_t* timep, struct tm* result) { errno_t ret = localtime_s(result, timep); return (ret == 0) ? result : NULL; } -} -using port::timeval; -using port::gettimeofday; -using port::localtime_r; +} // namespace port + } // namespace ROCKSDB_NAMESPACE #else #include #include + +namespace ROCKSDB_NAMESPACE { + +namespace port { + +using TimeVal = struct timeval; + +inline void GetTimeOfDay(TimeVal* tv, struct timezone* tz) { + gettimeofday(tv, tz); +} + +inline struct tm* LocalTimeR(const time_t* timep, struct tm* result) { + return localtime_r(timep, result); +} + +} // namespace port + +} // namespace ROCKSDB_NAMESPACE + #endif diff --git a/port/win/port_win.cc b/port/win/port_win.cc index 38f3b7db7..37e8f655c 100644 --- a/port/win/port_win.cc +++ b/port/win/port_win.cc @@ -52,7 +52,7 @@ std::wstring utf8_to_utf16(const std::string& utf8) { } #endif -void gettimeofday(struct timeval* tv, struct timezone* /* tz */) { +void GetTimeOfDay(TimeVal* tv, struct timezone* /* tz */) { std::chrono::microseconds usNow( std::chrono::duration_cast( std::chrono::system_clock::now().time_since_epoch())); diff --git a/port/win/win_logger.cc b/port/win/win_logger.cc index a45f3c6d4..6773699d1 100644 --- a/port/win/win_logger.cc +++ b/port/win/win_logger.cc @@ -118,8 +118,8 @@ void WinLogger::Logv(const char* format, va_list ap) { char* p = base; char* limit = base + bufsize; - struct timeval now_tv; - gettimeofday(&now_tv, nullptr); + port::TimeVal now_tv; + port::GetTimeOfDay(&now_tv, nullptr); const time_t seconds = now_tv.tv_sec; struct tm t; localtime_s(&t, &seconds); diff --git a/util/string_util.cc b/util/string_util.cc index 0a74c6966..94459dac4 100644 --- a/util/string_util.cc +++ b/util/string_util.cc @@ -150,7 +150,7 @@ std::string TimeToHumanString(int unixtime) { char time_buffer[80]; time_t rawtime = unixtime; struct tm tInfo; - struct tm* timeinfo = localtime_r(&rawtime, &tInfo); + struct tm* timeinfo = port::LocalTimeR(&rawtime, &tInfo); assert(timeinfo == &tInfo); strftime(time_buffer, 80, "%c", timeinfo); return std::string(time_buffer); diff --git a/utilities/persistent_cache/hash_table_bench.cc b/utilities/persistent_cache/hash_table_bench.cc index a1f05007e..74d7e2edf 100644 --- a/utilities/persistent_cache/hash_table_bench.cc +++ b/utilities/persistent_cache/hash_table_bench.cc @@ -11,14 +11,16 @@ int main() { fprintf(stderr, "Please install gflags to run tools\n"); } #else +#include +#include + #include #include #include #include -#include -#include #include "port/port_posix.h" +#include "port/sys_time.h" #include "rocksdb/env.h" #include "util/gflags_compat.h" #include "util/mutexlock.h" @@ -152,8 +154,8 @@ class HashTableBenchmark { } static uint64_t NowInMillSec() { - timeval tv; - gettimeofday(&tv, /*tz=*/nullptr); + port::TimeVal tv; + port::GetTimeOfDay(&tv, /*tz=*/nullptr); return tv.tv_sec * 1000 + tv.tv_usec / 1000; }