Skip to content

Commit

Permalink
fix: drop custom (v)snprintf definition (#992)
Browse files Browse the repository at this point in the history
The functions are available since C++11.
  • Loading branch information
sergiud authored Dec 20, 2023
1 parent 3731c12 commit 3fcf77a
Show file tree
Hide file tree
Showing 12 changed files with 23 additions and 64 deletions.
4 changes: 0 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion bazel/glog.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
]
Expand Down
3 changes: 0 additions & 3 deletions src/config.h.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -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 <dlfcn.h> header file. */
#cmakedefine HAVE_DLFCN_H

Expand Down
5 changes: 3 additions & 2 deletions src/googletest.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<void*>(nullptr));
snprintf(ptr_str, str_size, "%p", reinterpret_cast<void*>(PTR_TEST_VALUE));
std::snprintf(null_str, str_size, "%p", static_cast<void*>(nullptr));
std::snprintf(ptr_str, str_size, "%p",
reinterpret_cast<void*>(PTR_TEST_VALUE));

StringReplace(&line, "__NULLP__", null_str);
StringReplace(&line, "__PTRTEST__", ptr_str);
Expand Down
4 changes: 2 additions & 2 deletions src/logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
5 changes: 3 additions & 2 deletions src/logging_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -1435,7 +1435,8 @@ TEST(LogBacktraceAt, DoesBacktraceAtRightLineWhenEnabled) {
StrictMock<ScopedMockLog> 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
Expand Down
12 changes: 6 additions & 6 deletions src/raw_logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<size_t>(n) > *size) return false;
*size -= static_cast<size_t>(n);
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/stl_logging_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/symbolize.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions src/utilities.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,17 +120,17 @@ 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

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);
}

Expand Down Expand Up @@ -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
Expand Down
23 changes: 2 additions & 21 deletions src/windows/port.cc
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -38,20 +38,10 @@

#include "port.h"

#include <cstdarg> // for va_list, va_start, va_end
#include <ctime>

#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);
Expand All @@ -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
16 changes: 0 additions & 16 deletions src/windows/port.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
#include <winsock.h> /* for gethostname */

#include <cstdarg> /* template_dictionary.cc uses va_copy */
#include <cstdio> /* read in vsnprintf decl. before redefining it */
#include <cstring> /* for _strnicmp(), strerror_s() */
#include <ctime> /* for localtime_s() */
/* Note: the C++ #includes are all together at the bottom. This file is
Expand Down Expand Up @@ -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.
*/
Expand Down

0 comments on commit 3fcf77a

Please sign in to comment.