metrics library: convert to proper C++/libbase
This mostly converts fprintfs to proper logs, char* to
std::string wherever appropriate.
BUG=chromium:355796
TEST=unit tests
Change-Id: Ieb1cb110be5e281b7e0c764a0dfce895f33d4a3c
Reviewed-on: https://chromium-review.googlesource.com/199610
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
Commit-Queue: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
diff --git a/metrics/metrics_library.cc b/metrics/metrics_library.cc
index 32bacf6..97cb278 100644
--- a/metrics/metrics_library.cc
+++ b/metrics/metrics_library.cc
@@ -4,6 +4,8 @@
#include "metrics_library.h"
+#include <base/logging.h>
+#include <base/strings/stringprintf.h>
#include <errno.h>
#include <sys/file.h>
#include <sys/stat.h>
@@ -51,23 +53,6 @@
time_t MetricsLibrary::cached_enabled_time_ = 0;
bool MetricsLibrary::cached_enabled_ = false;
-using std::string;
-
-// TODO(sosa@chromium.org) - use Chromium logger instead of stderr
-static void PrintError(const char* message, const char* file,
- int code) {
- static const char kProgramName[] = "libmetrics";
- if (code == 0) {
- fprintf(stderr, "%s: %s\n", kProgramName, message);
- } else if (file == NULL) {
- fprintf(stderr, "%s: ", kProgramName);
- perror(message);
- } else {
- fprintf(stderr, "%s: %s: ", kProgramName, file);
- perror(message);
- }
-}
-
// Copied from libbase to avoid pulling in all of libbase just for libmetrics.
static int WriteFileDescriptor(const int fd, const char* data, int size) {
// Allow for partial writes.
@@ -84,10 +69,7 @@
return bytes_written_total;
}
-MetricsLibrary::MetricsLibrary()
- : uma_events_file_(NULL),
- consent_file_(kConsentFile) {}
-
+MetricsLibrary::MetricsLibrary() : consent_file_(kConsentFile) {}
MetricsLibrary::~MetricsLibrary() {}
// We take buffer and buffer_size as parameters in order to simplify testing
@@ -178,7 +160,7 @@
// still respect the consent file if it is present for migration purposes.
// TODO(pastarmovj)
if (!has_policy) {
- enabled = stat(consent_file_, &stat_buffer) >= 0;
+ enabled = stat(consent_file_.c_str(), &stat_buffer) >= 0;
}
if (enabled && !IsGuestMode())
@@ -189,14 +171,20 @@
return cached_enabled_;
}
-bool MetricsLibrary::SendMessageToChrome(int32_t length, const char* message) {
-
- int chrome_fd = HANDLE_EINTR(open(uma_events_file_,
+bool MetricsLibrary::SendMessageToChrome(const std::string& message) {
+ int size = static_cast<int>(message.size());
+ if (size > kBufferSize) {
+ LOG(ERROR) << "chrome message too big (" << size << " bytes)";
+ return false;
+ }
+ // Use libc here instead of chromium base classes because we need a UNIX fd
+ // for flock.
+ int chrome_fd = HANDLE_EINTR(open(uma_events_file_.c_str(),
O_WRONLY | O_APPEND | O_CREAT,
READ_WRITE_ALL_FILE_FLAGS));
// If we failed to open it, return.
if (chrome_fd < 0) {
- PrintError("open", uma_events_file_, errno);
+ PLOG(ERROR) << uma_events_file_ << ": open";
return false;
}
@@ -206,16 +194,16 @@
fchmod(chrome_fd, READ_WRITE_ALL_FILE_FLAGS);
// Grab an exclusive lock to protect Chrome from truncating
- // underneath us. Keep the file locked as briefly as possible.
+ // underneath us.
if (HANDLE_EINTR(flock(chrome_fd, LOCK_EX)) < 0) {
- PrintError("flock", uma_events_file_, errno);
+ PLOG(ERROR) << uma_events_file_ << ": flock";
IGNORE_EINTR(close(chrome_fd));
return false;
}
bool success = true;
- if (WriteFileDescriptor(chrome_fd, message, length) != length) {
- PrintError("write", uma_events_file_, errno);
+ if (WriteFileDescriptor(chrome_fd, message.c_str(), size) != size) {
+ PLOG(ERROR) << uma_events_file_ << ": write";
success = false;
}
@@ -224,44 +212,28 @@
return success;
}
-int32_t MetricsLibrary::FormatChromeMessage(int32_t buffer_size, char* buffer,
- const char* format, ...) {
- int32_t message_length;
- size_t len_size = sizeof(message_length);
-
- // Format the non-LENGTH contents in the buffer by leaving space for
- // LENGTH at the start of the buffer.
- va_list args;
- va_start(args, format);
- message_length = vsnprintf(&buffer[len_size], buffer_size - len_size,
- format, args);
- va_end(args);
-
- if (message_length < 0) {
- PrintError("chrome message format error", NULL, 0);
- return -1;
- }
-
- // +1 to account for the trailing \0.
- message_length += len_size + 1;
- if (message_length > buffer_size) {
- PrintError("chrome message too long", NULL, 0);
- return -1;
- }
-
- // Prepend LENGTH to the message.
- memcpy(buffer, &message_length, len_size);
- return message_length;
+const std::string MetricsLibrary::FormatChromeMessage(
+ const std::string& name,
+ const std::string& value) {
+ uint32 message_length =
+ sizeof(message_length) + name.size() + 1 + value.size() + 1;
+ std::string result;
+ result.reserve(message_length);
+ // Marshal the total message length in the native byte order.
+ result.assign(reinterpret_cast<char*>(&message_length),
+ sizeof(message_length));
+ result += name + '\0' + value + '\0';
+ return result;
}
void MetricsLibrary::Init() {
uma_events_file_ = kUMAEventsPath;
}
-bool MetricsLibrary::SendToAutotest(const string& name, int value) {
+bool MetricsLibrary::SendToAutotest(const std::string& name, int value) {
FILE* autotest_file = fopen(kAutotestPath, "a+");
if (autotest_file == NULL) {
- PrintError("fopen", kAutotestPath, errno);
+ PLOG(ERROR) << kAutotestPath << ": fopen";
return false;
}
@@ -270,74 +242,48 @@
return true;
}
-bool MetricsLibrary::SendToUMA(const string& name, int sample,
- int min, int max, int nbuckets) {
+bool MetricsLibrary::SendToUMA(const std::string& name,
+ int sample,
+ int min,
+ int max,
+ int nbuckets) {
// Format the message.
- char message[kBufferSize];
- int32_t message_length =
- FormatChromeMessage(kBufferSize, message,
- "histogram%c%s %d %d %d %d", '\0',
- name.c_str(), sample, min, max, nbuckets);
- if (message_length < 0)
- return false;
-
+ std::string value = base::StringPrintf("%s %d %d %d %d",
+ name.c_str(), sample, min, max, nbuckets);
+ std::string message = FormatChromeMessage("histogram", value);
// Send the message.
- return SendMessageToChrome(message_length, message);
+ return SendMessageToChrome(message);
}
bool MetricsLibrary::SendEnumToUMA(const std::string& name, int sample,
int max) {
// Format the message.
- char message[kBufferSize];
- int32_t message_length =
- FormatChromeMessage(kBufferSize, message,
- "linearhistogram%c%s %d %d", '\0',
- name.c_str(), sample, max);
- if (message_length < 0)
- return false;
-
+ std::string value = base::StringPrintf("%s %d %d", name.c_str(), sample, max);
+ std::string message = FormatChromeMessage("linearhistogram", value);
// Send the message.
- return SendMessageToChrome(message_length, message);
+ return SendMessageToChrome(message);
}
bool MetricsLibrary::SendSparseToUMA(const std::string& name, int sample) {
// Format the message.
- char message[kBufferSize];
- int32_t message_length =
- FormatChromeMessage(kBufferSize, message, "sparsehistogram%c%s %d",
- '\0', name.c_str(), sample);
- if (message_length < 0)
- return false;
-
+ std::string value = base::StringPrintf("%s %d", name.c_str(), sample);
+ std::string message = FormatChromeMessage("sparsehistogram", value);
// Send the message.
- return SendMessageToChrome(message_length, message);
+ return SendMessageToChrome(message);
}
bool MetricsLibrary::SendUserActionToUMA(const std::string& action) {
// Format the message.
- char message[kBufferSize];
- int32_t message_length =
- FormatChromeMessage(kBufferSize, message,
- "useraction%c%s", '\0', action.c_str());
- if (message_length < 0)
- return false;
-
+ std::string message = FormatChromeMessage("useraction", action);
// Send the message.
- return SendMessageToChrome(message_length, message);
+ return SendMessageToChrome(message);
}
bool MetricsLibrary::SendCrashToUMA(const char *crash_kind) {
// Format the message.
- char message[kBufferSize];
- int32_t message_length =
- FormatChromeMessage(kBufferSize, message,
- "crash%c%s", '\0', crash_kind);
-
- if (message_length < 0)
- return false;
-
+ std::string message = FormatChromeMessage("crash", crash_kind);
// Send the message.
- return SendMessageToChrome(message_length, message);
+ return SendMessageToChrome(message);
}
void MetricsLibrary::SetPolicyProvider(policy::PolicyProvider* provider) {