Merge "libbase: logging fixes"
diff --git a/base/logging.cpp b/base/logging.cpp
index 34229a2..e9e06df 100644
--- a/base/logging.cpp
+++ b/base/logging.cpp
@@ -70,9 +70,14 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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() {
diff --git a/base/logging_test.cpp b/base/logging_test.cpp
index 1a92c94..a1e31a6 100644
--- a/base/logging_test.cpp
+++ b/base/logging_test.cpp
@@ -76,10 +76,10 @@
const char* message) {
static const char* log_characters = "VDIWEF";
char log_char = log_characters[severity];
+ std::string holder(__FILE__);
return android::base::StringPrintf(
- "%c[[:space:]]+[[:digit:]]+[[:space:]]+[[:digit:]]+ " __FILE__
- ":[[:digit:]]+] %s",
- log_char, message);
+ "%c[[:space:]]+[[:digit:]]+[[:space:]]+[[:digit:]]+ %s:[[:digit:]]+] %s",
+ log_char, basename(&holder[0]), message);
}
TEST(logging, LOG) {
@@ -100,7 +100,7 @@
#if !defined(_WIN32)
std::regex message_regex(
make_log_pattern(android::base::WARNING, "foobar"));
- ASSERT_TRUE(std::regex_search(output, message_regex));
+ ASSERT_TRUE(std::regex_search(output, message_regex)) << output;
#endif
}
@@ -116,7 +116,7 @@
#if !defined(_WIN32)
std::regex message_regex(
make_log_pattern(android::base::INFO, "foobar"));
- ASSERT_TRUE(std::regex_search(output, message_regex));
+ ASSERT_TRUE(std::regex_search(output, message_regex)) << output;
#endif
}
@@ -143,7 +143,7 @@
#if !defined(_WIN32)
std::regex message_regex(
make_log_pattern(android::base::DEBUG, "foobar"));
- ASSERT_TRUE(std::regex_search(output, message_regex));
+ ASSERT_TRUE(std::regex_search(output, message_regex)) << output;
#endif
}
}
@@ -162,7 +162,7 @@
#if !defined(_WIN32)
std::regex message_regex(make_log_pattern(
android::base::INFO, "foobar: No such file or directory"));
- ASSERT_TRUE(std::regex_search(output, message_regex));
+ ASSERT_TRUE(std::regex_search(output, message_regex)) << output;
#endif
}
}
@@ -183,7 +183,7 @@
android::base::StringPrintf("%s unimplemented ", __PRETTY_FUNCTION__);
std::regex message_regex(
make_log_pattern(android::base::ERROR, expected_message.c_str()));
- ASSERT_TRUE(std::regex_search(output, message_regex));
+ ASSERT_TRUE(std::regex_search(output, message_regex)) << output;
#endif
}
}
