From 3fcf77a6915f34990397b4e5971485036c1c01ce Mon Sep 17 00:00:00 2001 From: Sergiu Deitsch Date: Wed, 20 Dec 2023 20:49:08 +0100 Subject: [PATCH] fix: drop custom (v)snprintf definition (#992) The functions are available since C++11. --- CMakeLists.txt | 4 ---- bazel/glog.bzl | 1 - src/config.h.cmake.in | 3 --- src/googletest.h | 5 +++-- src/logging.cc | 4 ++-- src/logging_unittest.cc | 5 +++-- src/raw_logging.cc | 12 ++++++------ src/stl_logging_unittest.cc | 2 +- src/symbolize.cc | 2 +- src/utilities.cc | 10 +++++----- src/windows/port.cc | 23 ++--------------------- src/windows/port.h | 16 ---------------- 12 files changed, 23 insertions(+), 64 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b78a854..1311b9e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -151,10 +151,6 @@ endif (Threads_FOUND) check_cxx_symbol_exists (pthread_threadid_np "pthread.h" HAVE_PTHREAD_THREADID_NP) cmake_pop_check_state () -# NOTE: Cannot use check_function_exists here since >=vc-14.0 can define -# snprintf as an inline function -check_cxx_symbol_exists (snprintf cstdio HAVE_SNPRINTF) - cmake_push_check_state (RESET) set (CMAKE_REQUIRED_LIBRARIES dbghelp) check_cxx_symbol_exists (UnDecorateSymbolName "windows.h;dbghelp.h" HAVE_DBGHELP) diff --git a/bazel/glog.bzl b/bazel/glog.bzl index 71556b7..8234d0f 100644 --- a/bazel/glog.bzl +++ b/bazel/glog.bzl @@ -113,7 +113,6 @@ def glog_library(namespace = "google", with_gflags = 1, **kwargs): # Override -DGLOG_EXPORT= from the cc_library's defines. "-DGLOG_EXPORT=__declspec(dllexport)", "-DGLOG_NO_ABBREVIATED_SEVERITIES", - "-DHAVE_SNPRINTF", "-DHAVE__CHSIZE_S", "-I" + src_windows, ] diff --git a/src/config.h.cmake.in b/src/config.h.cmake.in index 3fed2a6..7044097 100644 --- a/src/config.h.cmake.in +++ b/src/config.h.cmake.in @@ -7,9 +7,6 @@ /* Define if you have the `dladdr' function */ #cmakedefine HAVE_DLADDR -/* Define if you have the `snprintf' function */ -#cmakedefine HAVE_SNPRINTF - /* Define to 1 if you have the header file. */ #cmakedefine HAVE_DLFCN_H diff --git a/src/googletest.h b/src/googletest.h index 0f96099..329794e 100644 --- a/src/googletest.h +++ b/src/googletest.h @@ -487,8 +487,9 @@ static inline string Munge(const string& filename) { const size_t str_size = 256; char null_str[str_size]; char ptr_str[str_size]; - snprintf(null_str, str_size, "%p", static_cast(nullptr)); - snprintf(ptr_str, str_size, "%p", reinterpret_cast(PTR_TEST_VALUE)); + std::snprintf(null_str, str_size, "%p", static_cast(nullptr)); + std::snprintf(ptr_str, str_size, "%p", + reinterpret_cast(PTR_TEST_VALUE)); StringReplace(&line, "__NULLP__", null_str); StringReplace(&line, "__PTRTEST__", ptr_str); diff --git a/src/logging.cc b/src/logging.cc index 8d90e2b..4df6f4c 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -1699,7 +1699,7 @@ void LogMessage::Init(const char* file, if (!FLAGS_log_backtrace_at.empty()) { char fileline[128]; - snprintf(fileline, sizeof(fileline), "%s:%d", data_->basename_, line); + std::snprintf(fileline, sizeof(fileline), "%s:%d", data_->basename_, line); #ifdef HAVE_STACKTRACE if (FLAGS_log_backtrace_at == fileline) { string stacktrace; @@ -2614,7 +2614,7 @@ string StrError(int err) { char buf[100]; int rc = posix_strerror_r(err, buf, sizeof(buf)); if ((rc < 0) || (buf[0] == '\000')) { - snprintf(buf, sizeof(buf), "Error number %d", err); + std::snprintf(buf, sizeof(buf), "Error number %d", err); } return buf; } diff --git a/src/logging_unittest.cc b/src/logging_unittest.cc index 21232f6..0b43e42 100644 --- a/src/logging_unittest.cc +++ b/src/logging_unittest.cc @@ -1006,7 +1006,7 @@ static void TestTruncate() { int fd; CHECK_ERR(fd = open(path.c_str(), O_APPEND | O_WRONLY)); char fdpath[64]; - snprintf(fdpath, sizeof(fdpath), "/proc/self/fd/%d", fd); + std::snprintf(fdpath, sizeof(fdpath), "/proc/self/fd/%d", fd); TestOneTruncate(fdpath, 10, 10, 10, 10, 10); #endif @@ -1435,7 +1435,8 @@ TEST(LogBacktraceAt, DoesBacktraceAtRightLineWhenEnabled) { StrictMock log; char where[100]; - snprintf(where, 100, "%s:%d", const_basename(__FILE__), kBacktraceAtLine); + std::snprintf(where, 100, "%s:%d", const_basename(__FILE__), + kBacktraceAtLine); FLAGS_log_backtrace_at = where; // The LOG at the specified line should include a stacktrace which includes diff --git a/src/raw_logging.cc b/src/raw_logging.cc index 8debc18..ede534d 100644 --- a/src/raw_logging.cc +++ b/src/raw_logging.cc @@ -82,10 +82,10 @@ _START_GOOGLE_NAMESPACE_ #define GLOG_ATTRIBUTE_FORMAT_ARG(stringIndex) #endif -// CAVEAT: vsnprintf called from *DoRawLog below has some (exotic) code paths -// that invoke malloc() and getenv() that might acquire some locks. -// If this becomes a problem we should reimplement a subset of vsnprintf -// that does not need locks and malloc. +// CAVEAT: std::vsnprintf called from *DoRawLog below has some (exotic) code +// paths that invoke malloc() and getenv() that might acquire some locks. If +// this becomes a problem we should reimplement a subset of std::vsnprintf that +// does not need locks and malloc. // Helper for RawLog__ below. // *DoRawLog writes to *buf of *size and move them past the written portion. @@ -94,7 +94,7 @@ GLOG_ATTRIBUTE_FORMAT(printf, 3, 4) static bool DoRawLog(char** buf, size_t* size, const char* format, ...) { va_list ap; va_start(ap, format); - int n = vsnprintf(*buf, *size, format, ap); + int n = std::vsnprintf(*buf, *size, format, ap); va_end(ap); if (n < 0 || static_cast(n) > *size) return false; *size -= static_cast(n); @@ -109,7 +109,7 @@ inline static bool VADoRawLog(char** buf, size_t* size, #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wformat-nonliteral" #endif - int n = vsnprintf(*buf, *size, format, ap); + int n = std::vsnprintf(*buf, *size, format, ap); #if defined(__GNUC__) #pragma GCC diagnostic pop #endif diff --git a/src/stl_logging_unittest.cc b/src/stl_logging_unittest.cc index 44aba54..4cb7c23 100644 --- a/src/stl_logging_unittest.cc +++ b/src/stl_logging_unittest.cc @@ -82,7 +82,7 @@ static void TestSTLLogging() { if (i > 0) expected += ' '; const size_t buf_size = 256; char buf[buf_size]; - snprintf(buf, buf_size, "%d", i); + std::snprintf(buf, buf_size, "%d", i); expected += buf; } v.push_back(100); diff --git a/src/symbolize.cc b/src/symbolize.cc index 8e84361..f1edd65 100644 --- a/src/symbolize.cc +++ b/src/symbolize.cc @@ -391,7 +391,7 @@ struct FileDescriptor { // // Note: we don't use ProcMapsIterator since the object is big (it has // a 5k array member) and uses async-unsafe functions such as sscanf() -// and snprintf(). +// and std::snprintf(). class LineReader { public: explicit LineReader(int fd, char *buf, size_t buf_len, size_t offset) diff --git a/src/utilities.cc b/src/utilities.cc index f825470..5039ae6 100644 --- a/src/utilities.cc +++ b/src/utilities.cc @@ -120,8 +120,8 @@ static void DumpPCAndSymbol(DebugWriter *writerfn, void *arg, void *pc, symbol = tmp; } char buf[1024]; - snprintf(buf, sizeof(buf), "%s@ %*p %s\n", - prefix, kPrintfPointerFieldWidth, pc, symbol); + std::snprintf(buf, sizeof(buf), "%s@ %*p %s\n", prefix, + kPrintfPointerFieldWidth, pc, symbol); writerfn(buf, arg); } #endif @@ -129,8 +129,8 @@ static void DumpPCAndSymbol(DebugWriter *writerfn, void *arg, void *pc, static void DumpPC(DebugWriter *writerfn, void *arg, void *pc, const char * const prefix) { char buf[100]; - snprintf(buf, sizeof(buf), "%s@ %*p\n", - prefix, kPrintfPointerFieldWidth, pc); + std::snprintf(buf, sizeof(buf), "%s@ %*p\n", prefix, kPrintfPointerFieldWidth, + pc); writerfn(buf, arg); } @@ -334,7 +334,7 @@ static void MyUserNameInitializer() { if (pwuid_res == 0 && result) { g_my_user_name = pwd.pw_name; } else { - snprintf(buffer, sizeof(buffer), "uid%d", uid); + std::snprintf(buffer, sizeof(buffer), "uid%d", uid); g_my_user_name = buffer; } #endif diff --git a/src/windows/port.cc b/src/windows/port.cc index f80a54a..9631bed 100755 --- a/src/windows/port.cc +++ b/src/windows/port.cc @@ -1,4 +1,4 @@ -/* Copyright (c) 2008, Google Inc. +/* Copyright (c) 2023, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -38,20 +38,10 @@ #include "port.h" -#include // for va_list, va_start, va_end #include #include "config.h" -// These call the windows _vsnprintf, but always NUL-terminate. -int safe_vsnprintf(char* str, std::size_t size, const char* format, - va_list ap) { - if (size == 0) // not even room for a \0? - return -1; // not what C99 says to do, but what windows does - str[size-1] = '\0'; - return _vsnprintf(str, size-1, format, ap); -} - #ifndef HAVE_LOCALTIME_R struct tm* localtime_r(const std::time_t* timep, std::tm* result) { localtime_s(result, timep); @@ -63,13 +53,4 @@ struct tm* gmtime_r(const std::time_t* timep, std::tm* result) { gmtime_s(result, timep); return result; } -#endif // not HAVE_GMTIME_R -#ifndef HAVE_SNPRINTF -int snprintf(char *str, size_t size, const char *format, ...) { - va_list ap; - va_start(ap, format); - const int r = vsnprintf(str, size, format, ap); - va_end(ap); - return r; -} -#endif +#endif // not HAVE_GMTIME_R diff --git a/src/windows/port.h b/src/windows/port.h index bedc2e8..52662f3 100755 --- a/src/windows/port.h +++ b/src/windows/port.h @@ -56,7 +56,6 @@ #include /* for gethostname */ #include /* template_dictionary.cc uses va_copy */ -#include /* read in vsnprintf decl. before redefining it */ #include /* for _strnicmp(), strerror_s() */ #include /* for localtime_s() */ /* Note: the C++ #includes are all together at the bottom. This file is @@ -111,21 +110,6 @@ enum { STDIN_FILENO = 0, STDOUT_FILENO = 1, STDERR_FILENO = 2 }; /* Sleep is in ms, on windows */ #define sleep(secs) Sleep((secs) * 1000) -/* We can't just use _vsnprintf and _snprintf as drop-in-replacements, - * because they don't always NUL-terminate. :-( We also can't use the - * name vsnprintf, since windows defines that (but not snprintf (!)). - */ -#ifndef HAVE_SNPRINTF -extern int GLOG_EXPORT snprintf(char* str, size_t size, const char* format, - ...); -#endif -extern int GLOG_EXPORT safe_vsnprintf(char* str, size_t size, - const char* format, va_list ap); -#define vsnprintf(str, size, format, ap) safe_vsnprintf(str, size, format, ap) -#ifndef va_copy -#define va_copy(dst, src) (dst) = (src) -#endif - /* Windows doesn't support specifying the number of buckets as a * hash_map constructor arg, so we leave this blank. */