diff options
author | Spencer Low <CompareAndSwap@gmail.com> | 2015-08-11 16:00:13 -0700 |
---|---|---|
committer | Elliott Hughes <enh@google.com> | 2015-08-13 14:08:31 -0700 |
commit | bdab59a8613c732df5d746328ff2268be8da13ab (patch) | |
tree | 3a6c5abbafd4ee588760ecd50b9b2b544f10d2e8 /base/logging.cpp | |
parent | 57532b2a067082fa5968094c2c615f1832fa1971 (diff) | |
download | core-bdab59a8613c732df5d746328ff2268be8da13ab.tar.gz core-bdab59a8613c732df5d746328ff2268be8da13ab.tar.bz2 core-bdab59a8613c732df5d746328ff2268be8da13ab.zip |
libbase: logging fixes
Win32:
- getprogname(): call basename() which is available in mingw's crt.
Don't potentially go recursive with DCHECK_GT().
- Use Win32 critical section instead of mutex.
Other:
- Change log_characters check to compile-time.
- Fix code that gets the basename of __FILE__. The previous code was not
setting _file, so it didn't work.
- Save and restore errno for LOG calls. Inspired by similar Chromium code.
Change-Id: Ie7bb700918be726fa81d60177d1894d2daeff296
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
Diffstat (limited to 'base/logging.cpp')
-rw-r--r-- | base/logging.cpp | 62 |
1 files changed, 43 insertions, 19 deletions
diff --git a/base/logging.cpp b/base/logging.cpp index 34229a2f8..e9e06dfc4 100644 --- a/base/logging.cpp +++ b/base/logging.cpp @@ -70,9 +70,14 @@ const char* getprogname() { static char progname[MAX_PATH] = {}; if (first) { - // TODO(danalbert): This is a full path on Windows. Just get the basename. - DWORD nchars = GetModuleFileName(nullptr, progname, sizeof(progname)); - DCHECK_GT(nchars, 0U); + CHAR longname[MAX_PATH]; + DWORD nchars = GetModuleFileNameA(nullptr, longname, arraysize(longname)); + if ((nchars >= arraysize(longname)) || (nchars == 0)) { + // String truncation or some other error. + strcpy(progname, "<unknown>"); + } else { + strcpy(progname, basename(longname)); + } first = false; } @@ -82,25 +87,22 @@ const char* getprogname() { class mutex { public: mutex() { - semaphore_ = CreateSemaphore(nullptr, 1, 1, nullptr); - CHECK(semaphore_ != nullptr) << "Failed to create Mutex"; + InitializeCriticalSection(&critical_section_); } ~mutex() { - CloseHandle(semaphore_); + DeleteCriticalSection(&critical_section_); } void lock() { - DWORD result = WaitForSingleObject(semaphore_, INFINITE); - CHECK_EQ(result, WAIT_OBJECT_0) << GetLastError(); + EnterCriticalSection(&critical_section_); } void unlock() { - bool result = ReleaseSemaphore(semaphore_, 1, nullptr); - CHECK(result); + LeaveCriticalSection(&critical_section_); } private: - HANDLE semaphore_; + CRITICAL_SECTION critical_section_; }; template <typename LockT> @@ -147,8 +149,9 @@ static const char* ProgramInvocationName() { void StderrLogger(LogId, LogSeverity severity, const char*, const char* file, unsigned int line, const char* message) { - static const char* log_characters = "VDIWEF"; - CHECK_EQ(strlen(log_characters), FATAL + 1U); + static const char log_characters[] = "VDIWEF"; + static_assert(arraysize(log_characters) - 1 == FATAL + 1, + "Mismatch in size of log_characters and values in LogSeverity"); char severity_char = log_characters[severity]; fprintf(stderr, "%s %c %5d %5d %s:%u] %s\n", ProgramInvocationName(), severity_char, getpid(), gettid(), file, line, message); @@ -197,6 +200,20 @@ void InitLogging(char* argv[], LogFunction&& logger) { InitLogging(argv); } +// TODO: make this public; it's independently useful. +class ErrnoRestorer { + public: + ErrnoRestorer(int saved_errno) : saved_errno_(saved_errno) { + } + + ~ErrnoRestorer() { + errno = saved_errno_; + } + + private: + const int saved_errno_; +}; + void InitLogging(char* argv[]) { if (gInitialized) { return; @@ -257,19 +274,25 @@ void SetLogger(LogFunction&& logger) { gLogger = std::move(logger); } +// We can't use basename(3) because this code runs on the Mac, which doesn't +// have a non-modifying basename. +static const char* GetFileBasename(const char* file) { + const char* last_slash = strrchr(file, '/'); + return (last_slash == nullptr) ? file : last_slash + 1; +} + // This indirection greatly reduces the stack impact of having lots of // checks/logging in a function. class LogMessageData { public: LogMessageData(const char* file, unsigned int line, LogId id, - LogSeverity severity, int error) - : file_(file), + LogSeverity severity, int error, int saved_errno) + : file_(GetFileBasename(file)), line_number_(line), id_(id), severity_(severity), - error_(error) { - const char* last_slash = strrchr(file, '/'); - file = (last_slash == nullptr) ? file : last_slash + 1; + error_(error), + errno_restorer_(saved_errno) { } const char* GetFile() const { @@ -307,13 +330,14 @@ class LogMessageData { const LogId id_; const LogSeverity severity_; const int error_; + ErrnoRestorer errno_restorer_; DISALLOW_COPY_AND_ASSIGN(LogMessageData); }; LogMessage::LogMessage(const char* file, unsigned int line, LogId id, LogSeverity severity, int error) - : data_(new LogMessageData(file, line, id, severity, error)) { + : data_(new LogMessageData(file, line, id, severity, error, errno)) { } LogMessage::~LogMessage() { |