Merge "adb: don't divide by zero"
diff --git a/adb/adb_listeners.cpp b/adb/adb_listeners.cpp
index d5b1fd5..1b75090 100644
--- a/adb/adb_listeners.cpp
+++ b/adb/adb_listeners.cpp
@@ -33,21 +33,17 @@
};
static void ss_listener_event_func(int _fd, unsigned ev, void *_l) {
- asocket *s;
-
- if(ev & FDE_READ) {
+ if (ev & FDE_READ) {
struct sockaddr addr;
- socklen_t alen;
- int fd;
+ socklen_t alen = sizeof(addr);
+ int fd = adb_socket_accept(_fd, &addr, &alen);
+ if (fd < 0) return;
- alen = sizeof(addr);
- fd = adb_socket_accept(_fd, &addr, &alen);
- if(fd < 0) return;
+ int rcv_buf_size = CHUNK_SIZE;
+ adb_setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &rcv_buf_size, sizeof(rcv_buf_size));
- adb_socket_setbufsize(fd, CHUNK_SIZE);
-
- s = create_local_socket(fd);
- if(s) {
+ asocket* s = create_local_socket(fd);
+ if (s) {
connect_to_smartsocket(s);
return;
}
diff --git a/adb/file_sync_client.cpp b/adb/file_sync_client.cpp
index c422b4f..9ad7bad 100644
--- a/adb/file_sync_client.cpp
+++ b/adb/file_sync_client.cpp
@@ -103,7 +103,7 @@
// Sending header, payload, and footer in a single write makes a huge
// difference to "adb sync" performance.
bool SendSmallFile(const char* path_and_mode,
- const char* rpath,
+ const char* lpath, const char* rpath,
unsigned mtime,
const char* data, size_t data_length) {
Print(rpath);
@@ -139,8 +139,7 @@
req_done->path_length = mtime;
p += sizeof(SyncRequest);
- if (!WriteFdExactly(fd, &buf[0], (p - &buf[0]))) return false;
-
+ WriteOrDie(lpath, rpath, &buf[0], (p - &buf[0]));
total_bytes += data_length;
return true;
}
@@ -164,31 +163,27 @@
int lfd = adb_open(lpath, O_RDONLY);
if (lfd < 0) {
- Error("cannot open '%s': %s", lpath, strerror(errno));
+ Error("opening '%s' locally failed: %s", lpath, strerror(errno));
return false;
}
syncsendbuf sbuf;
sbuf.id = ID_DATA;
while (true) {
- int ret = adb_read(lfd, sbuf.data, max);
- if (ret <= 0) {
- if (ret < 0) {
- Error("cannot read '%s': %s", lpath, strerror(errno));
- adb_close(lfd);
- return false;
- }
+ int bytes_read = adb_read(lfd, sbuf.data, max);
+ if (bytes_read == -1) {
+ Error("reading '%s' locally failed: %s", lpath, strerror(errno));
+ adb_close(lfd);
+ return false;
+ } else if (bytes_read == 0) {
break;
}
- sbuf.size = ret;
- if (!WriteFdExactly(fd, &sbuf, sizeof(unsigned) * 2 + ret)) {
- adb_close(lfd);
- return false;
- }
- total_bytes += ret;
+ sbuf.size = bytes_read;
+ WriteOrDie(lpath, rpath, &sbuf, sizeof(SyncRequest) + bytes_read);
- bytes_copied += ret;
+ total_bytes += bytes_read;
+ bytes_copied += bytes_read;
if (total_size == 0) {
// This case can happen if we're racing against something that wrote to the file
@@ -206,12 +201,7 @@
syncmsg msg;
msg.data.id = ID_DONE;
msg.data.size = mtime;
- if (!WriteFdExactly(fd, &msg.data, sizeof(msg.data))) {
- Error("failed to send ID_DONE message for '%s': %s", rpath, strerror(errno));
- return false;
- }
-
- return true;
+ return WriteOrDie(lpath, rpath, &msg.data, sizeof(msg.data));
}
bool CopyDone(const char* from, const char* to) {
@@ -304,6 +294,27 @@
return SendRequest(ID_QUIT, ""); // TODO: add a SendResponse?
}
+ bool WriteOrDie(const char* from, const char* to, const void* data, size_t data_length) {
+ if (!WriteFdExactly(fd, data, data_length)) {
+ if (errno == ECONNRESET) {
+ // Assume adbd told us why it was closing the connection, and
+ // try to read failure reason from adbd.
+ syncmsg msg;
+ if (!ReadFdExactly(fd, &msg.status, sizeof(msg.status))) {
+ Error("failed to copy '%s' to '%s': no response: %s", from, to, strerror(errno));
+ } else if (msg.status.id != ID_FAIL) {
+ Error("failed to copy '%s' to '%s': not ID_FAIL: %d", from, to, msg.status.id);
+ } else {
+ ReportCopyFailure(from, to, msg);
+ }
+ } else {
+ Error("%zu-byte write failed: %s", data_length, strerror(errno));
+ }
+ _exit(1);
+ }
+ return true;
+ }
+
static uint64_t CurrentTimeMs() {
struct timeval tv;
gettimeofday(&tv, 0); // (Not clock_gettime because of Mac/Windows.)
@@ -369,7 +380,9 @@
}
buf[data_length++] = '\0';
- if (!sc.SendSmallFile(path_and_mode.c_str(), rpath, mtime, buf, data_length)) return false;
+ if (!sc.SendSmallFile(path_and_mode.c_str(), lpath, rpath, mtime, buf, data_length)) {
+ return false;
+ }
return sc.CopyDone(lpath, rpath);
#endif
}
@@ -390,7 +403,8 @@
sc.Error("failed to read all of '%s': %s", lpath, strerror(errno));
return false;
}
- if (!sc.SendSmallFile(path_and_mode.c_str(), rpath, mtime, data.data(), data.size())) {
+ if (!sc.SendSmallFile(path_and_mode.c_str(), lpath, rpath, mtime,
+ data.data(), data.size())) {
return false;
}
} else {
@@ -565,11 +579,12 @@
} else {
if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode)) {
sc.Error("skipping special file '%s'", lpath.c_str());
+ ci.skip = true;
} else {
ci.time = st.st_mtime;
ci.size = st.st_size;
- filelist->push_back(ci);
}
+ filelist->push_back(ci);
}
}
@@ -736,8 +751,7 @@
bool empty_dir = true;
// Put the files/dirs in rpath on the lists.
- auto callback = [&](unsigned mode, unsigned size, unsigned time,
- const char* name) {
+ auto callback = [&](unsigned mode, unsigned size, unsigned time, const char* name) {
if (IsDotOrDotDot(name)) {
return;
}
@@ -748,12 +762,18 @@
copyinfo ci = mkcopyinfo(lpath, rpath, name, mode);
if (S_ISDIR(mode)) {
dirlist.push_back(ci);
- } else if (S_ISREG(mode) || S_ISLNK(mode)) {
- ci.time = time;
- ci.size = size;
- filelist->push_back(ci);
} else {
- sc.Warning("skipping special file '%s'\n", name);
+ if (S_ISREG(mode)) {
+ ci.time = time;
+ ci.size = size;
+ } else if (S_ISLNK(mode)) {
+ sc.Warning("skipping symlink '%s'", name);
+ ci.skip = true;
+ } else {
+ sc.Warning("skipping special file '%s'", name);
+ ci.skip = true;
+ }
+ filelist->push_back(ci);
}
};
diff --git a/adb/file_sync_service.cpp b/adb/file_sync_service.cpp
index 7484a7c..945fa5a 100644
--- a/adb/file_sync_service.cpp
+++ b/adb/file_sync_service.cpp
@@ -394,6 +394,18 @@
void file_sync_service(int fd, void* cookie) {
std::vector<char> buffer(SYNC_DATA_MAX);
+ // If there's a problem on the device, we'll send an ID_FAIL message and
+ // close the socket. Unfortunately the kernel will sometimes throw that
+ // data away if the other end keeps writing without reading (which is
+ // the normal case with our protocol --- they won't read until the end).
+ // So set SO_LINGER to give the client 20s to get around to reading our
+ // failure response. Without this, the other side's ability to report
+ // useful errors is reduced.
+ struct linger l;
+ l.l_onoff = 1;
+ l.l_linger = 20;
+ adb_setsockopt(fd, SOL_SOCKET, SO_LINGER, &l, sizeof(l));
+
while (handle_sync_command(fd, buffer)) {
}
diff --git a/adb/sysdeps.h b/adb/sysdeps.h
index cba66fc..22c9b39 100644
--- a/adb/sysdeps.h
+++ b/adb/sysdeps.h
@@ -242,18 +242,6 @@
#undef setsockopt
#define setsockopt ___xxx_setsockopt
-static __inline__ int adb_socket_setbufsize( int fd, int bufsize )
-{
- int opt = bufsize;
- return adb_setsockopt(fd, SOL_SOCKET, SO_RCVBUF, (const void*)&opt, sizeof(opt));
-}
-
-static __inline__ void disable_tcp_nagle( int fd )
-{
- int on = 1;
- adb_setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, (const void*)&on, sizeof(on));
-}
-
extern int adb_socketpair( int sv[2] );
static __inline__ int adb_is_absolute_host_path(const char* path) {
@@ -670,18 +658,6 @@
#endif
}
-static __inline__ int adb_socket_setbufsize(int fd, int bufsize )
-{
- int opt = bufsize;
- return setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &opt, sizeof(opt));
-}
-
-static __inline__ void disable_tcp_nagle(int fd)
-{
- int on = 1;
- setsockopt( fd, IPPROTO_TCP, TCP_NODELAY, (void*)&on, sizeof(on) );
-}
-
static __inline__ int adb_setsockopt( int fd, int level, int optname, const void* optval, socklen_t optlen )
{
return setsockopt( fd, level, optname, optval, optlen );
@@ -739,4 +715,9 @@
#endif /* !_WIN32 */
+static inline void disable_tcp_nagle(int fd) {
+ int off = 1;
+ adb_setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &off, sizeof(off));
+}
+
#endif /* _ADB_SYSDEPS_H */
diff --git a/libbacktrace/Backtrace.cpp b/libbacktrace/Backtrace.cpp
index 9ead452..555e8cf 100644
--- a/libbacktrace/Backtrace.cpp
+++ b/libbacktrace/Backtrace.cpp
@@ -93,16 +93,26 @@
}
std::string Backtrace::FormatFrameData(const backtrace_frame_data_t* frame) {
- const char* map_name;
- if (BacktraceMap::IsValid(frame->map) && !frame->map.name.empty()) {
- map_name = frame->map.name.c_str();
+ uintptr_t relative_pc;
+ std::string map_name;
+ if (BacktraceMap::IsValid(frame->map)) {
+ relative_pc = BacktraceMap::GetRelativePc(frame->map, frame->pc);
+ if (!frame->map.name.empty()) {
+ map_name = frame->map.name.c_str();
+ if (map_name[0] == '[' && map_name[map_name.size() - 1] == ']') {
+ map_name.resize(map_name.size() - 1);
+ map_name += StringPrintf(":%" PRIPTR "]", frame->map.start);
+ }
+ } else {
+ map_name = StringPrintf("<anonymous:%" PRIPTR ">", frame->map.start);
+ }
} else {
map_name = "<unknown>";
+ relative_pc = frame->pc;
}
- uintptr_t relative_pc = BacktraceMap::GetRelativePc(frame->map, frame->pc);
-
- std::string line(StringPrintf("#%02zu pc %" PRIPTR " %s", frame->num, relative_pc, map_name));
+ std::string line(StringPrintf("#%02zu pc %" PRIPTR " ", frame->num, relative_pc));
+ line += map_name;
// Special handling for non-zero offset maps, we need to print that
// information.
if (frame->map.offset != 0) {
diff --git a/libbacktrace/backtrace_test.cpp b/libbacktrace/backtrace_test.cpp
index 9ebd639..ce04817 100644
--- a/libbacktrace/backtrace_test.cpp
+++ b/libbacktrace/backtrace_test.cpp
@@ -775,16 +775,29 @@
backtrace->FormatFrameData(&frame));
// Check map name empty, but exists.
- frame.map.start = 1;
- frame.map.end = 1;
+ frame.pc = 0xb0020;
+ frame.map.start = 0xb0000;
+ frame.map.end = 0xbffff;
frame.map.load_base = 0;
#if defined(__LP64__)
- EXPECT_EQ("#01 pc 0000000000000001 <unknown>",
+ EXPECT_EQ("#01 pc 0000000000000020 <anonymous:00000000000b0000>",
#else
- EXPECT_EQ("#01 pc 00000001 <unknown>",
+ EXPECT_EQ("#01 pc 00000020 <anonymous:000b0000>",
#endif
backtrace->FormatFrameData(&frame));
+ // Check map name begins with a [.
+ frame.pc = 0xc0020;
+ frame.map.start = 0xc0000;
+ frame.map.end = 0xcffff;
+ frame.map.load_base = 0;
+ frame.map.name = "[anon:thread signal stack]";
+#if defined(__LP64__)
+ EXPECT_EQ("#01 pc 0000000000000020 [anon:thread signal stack:00000000000c0000]",
+#else
+ EXPECT_EQ("#01 pc 00000020 [anon:thread signal stack:000c0000]",
+#endif
+ backtrace->FormatFrameData(&frame));
// Check relative pc is set and map name is set.
frame.pc = 0x12345679;
diff --git a/libcutils/fs_config.c b/libcutils/fs_config.c
index d4994db..6d50adc 100644
--- a/libcutils/fs_config.c
+++ b/libcutils/fs_config.c
@@ -132,11 +132,10 @@
{ 00750, AID_ROOT, AID_SHELL, 0, "data/nativetest/*" },
{ 00750, AID_ROOT, AID_SHELL, 0, "data/nativetest64/*" },
- /* the following three files are INTENTIONALLY set-uid, but they
+ /* the following two files are INTENTIONALLY set-uid, but they
* are NOT included on user builds. */
{ 04750, AID_ROOT, AID_SHELL, 0, "system/xbin/su" },
{ 06755, AID_ROOT, AID_ROOT, 0, "system/xbin/procmem" },
- { 04770, AID_ROOT, AID_RADIO, 0, "system/bin/pppd-ril" },
/* the following files have enhanced capabilities and ARE included in user builds. */
{ 00750, AID_ROOT, AID_SHELL, CAP_MASK_LONG(CAP_SETUID) | CAP_MASK_LONG(CAP_SETGID), "system/bin/run-as" },
diff --git a/liblog/Android.bp b/liblog/Android.bp
index 34e7f92..878feb8 100644
--- a/liblog/Android.bp
+++ b/liblog/Android.bp
@@ -52,6 +52,7 @@
},
windows: {
srcs: ["uio.c"],
+ enabled: true,
},
not_windows: {
srcs: ["event_tag_map.c"],
diff --git a/metricsd/constants.h b/metricsd/constants.h
index 3a7569b..ee0c9cb 100644
--- a/metricsd/constants.h
+++ b/metricsd/constants.h
@@ -18,7 +18,10 @@
#define METRICS_CONSTANTS_H_
namespace metrics {
-static const char kMetricsDirectory[] = "/data/misc/metrics/";
+static const char kSharedMetricsDirectory[] = "/data/misc/metrics/";
+static const char kMetricsdDirectory[] = "/data/misc/metricsd/";
+static const char kMetricsCollectorDirectory[] =
+ "/data/misc/metrics_collector/";
static const char kMetricsEventsFileName[] = "uma-events";
static const char kMetricsGUIDFileName[] = "Sysinfo.GUID";
static const char kMetricsServer[] = "https://clients4.google.com/uma/v2";
diff --git a/metricsd/metrics_client.cc b/metricsd/metrics_client.cc
index 78174ef..946b36a 100644
--- a/metricsd/metrics_client.cc
+++ b/metricsd/metrics_client.cc
@@ -140,8 +140,8 @@
}
static int DumpLogs() {
- base::FilePath events_file = base::FilePath(
- metrics::kMetricsDirectory).Append(metrics::kMetricsEventsFileName);
+ base::FilePath events_file = base::FilePath(metrics::kSharedMetricsDirectory)
+ .Append(metrics::kMetricsEventsFileName);
printf("Metrics from %s\n\n", events_file.value().data());
ScopedVector<metrics::MetricSample> metrics;
diff --git a/metricsd/metrics_collector.cc b/metricsd/metrics_collector.cc
index 28194a1..28f9ad3 100644
--- a/metricsd/metrics_collector.cc
+++ b/metricsd/metrics_collector.cc
@@ -149,51 +149,54 @@
return version_hash;
}
-void MetricsCollector::Init(bool testing,
- MetricsLibraryInterface* metrics_lib,
- const string& diskstats_path,
- const base::FilePath& metrics_directory) {
+void MetricsCollector::Init(bool testing, MetricsLibraryInterface* metrics_lib,
+ const string& diskstats_path,
+ const base::FilePath& private_metrics_directory,
+ const base::FilePath& shared_metrics_directory) {
CHECK(metrics_lib);
testing_ = testing;
- metrics_directory_ = metrics_directory;
+ shared_metrics_directory_ = shared_metrics_directory;
metrics_lib_ = metrics_lib;
- daily_active_use_.reset(
- new PersistentInteger("Platform.UseTime.PerDay"));
- version_cumulative_active_use_.reset(
- new PersistentInteger("Platform.CumulativeUseTime"));
- version_cumulative_cpu_use_.reset(
- new PersistentInteger("Platform.CumulativeCpuTime"));
+ daily_active_use_.reset(new PersistentInteger("Platform.UseTime.PerDay",
+ private_metrics_directory));
+ version_cumulative_active_use_.reset(new PersistentInteger(
+ "Platform.CumulativeUseTime", private_metrics_directory));
+ version_cumulative_cpu_use_.reset(new PersistentInteger(
+ "Platform.CumulativeCpuTime", private_metrics_directory));
- kernel_crash_interval_.reset(
- new PersistentInteger("Platform.KernelCrashInterval"));
- unclean_shutdown_interval_.reset(
- new PersistentInteger("Platform.UncleanShutdownInterval"));
- user_crash_interval_.reset(
- new PersistentInteger("Platform.UserCrashInterval"));
+ kernel_crash_interval_.reset(new PersistentInteger(
+ "Platform.KernelCrashInterval", private_metrics_directory));
+ unclean_shutdown_interval_.reset(new PersistentInteger(
+ "Platform.UncleanShutdownInterval", private_metrics_directory));
+ user_crash_interval_.reset(new PersistentInteger("Platform.UserCrashInterval",
+ private_metrics_directory));
- any_crashes_daily_count_.reset(
- new PersistentInteger("Platform.AnyCrashes.PerDay"));
- any_crashes_weekly_count_.reset(
- new PersistentInteger("Platform.AnyCrashes.PerWeek"));
- user_crashes_daily_count_.reset(
- new PersistentInteger("Platform.UserCrashes.PerDay"));
- user_crashes_weekly_count_.reset(
- new PersistentInteger("Platform.UserCrashes.PerWeek"));
- kernel_crashes_daily_count_.reset(
- new PersistentInteger("Platform.KernelCrashes.PerDay"));
- kernel_crashes_weekly_count_.reset(
- new PersistentInteger("Platform.KernelCrashes.PerWeek"));
- kernel_crashes_version_count_.reset(
- new PersistentInteger("Platform.KernelCrashesSinceUpdate"));
- unclean_shutdowns_daily_count_.reset(
- new PersistentInteger("Platform.UncleanShutdown.PerDay"));
- unclean_shutdowns_weekly_count_.reset(
- new PersistentInteger("Platform.UncleanShutdowns.PerWeek"));
+ any_crashes_daily_count_.reset(new PersistentInteger(
+ "Platform.AnyCrashes.PerDay", private_metrics_directory));
+ any_crashes_weekly_count_.reset(new PersistentInteger(
+ "Platform.AnyCrashes.PerWeek", private_metrics_directory));
+ user_crashes_daily_count_.reset(new PersistentInteger(
+ "Platform.UserCrashes.PerDay", private_metrics_directory));
+ user_crashes_weekly_count_.reset(new PersistentInteger(
+ "Platform.UserCrashes.PerWeek", private_metrics_directory));
+ kernel_crashes_daily_count_.reset(new PersistentInteger(
+ "Platform.KernelCrashes.PerDay", private_metrics_directory));
+ kernel_crashes_weekly_count_.reset(new PersistentInteger(
+ "Platform.KernelCrashes.PerWeek", private_metrics_directory));
+ kernel_crashes_version_count_.reset(new PersistentInteger(
+ "Platform.KernelCrashesSinceUpdate", private_metrics_directory));
+ unclean_shutdowns_daily_count_.reset(new PersistentInteger(
+ "Platform.UncleanShutdown.PerDay", private_metrics_directory));
+ unclean_shutdowns_weekly_count_.reset(new PersistentInteger(
+ "Platform.UncleanShutdowns.PerWeek", private_metrics_directory));
- daily_cycle_.reset(new PersistentInteger("daily.cycle"));
- weekly_cycle_.reset(new PersistentInteger("weekly.cycle"));
- version_cycle_.reset(new PersistentInteger("version.cycle"));
+ daily_cycle_.reset(
+ new PersistentInteger("daily.cycle", private_metrics_directory));
+ weekly_cycle_.reset(
+ new PersistentInteger("weekly.cycle", private_metrics_directory));
+ version_cycle_.reset(
+ new PersistentInteger("version.cycle", private_metrics_directory));
disk_usage_collector_.reset(new DiskUsageCollector(metrics_lib_));
averaged_stats_collector_.reset(
@@ -288,8 +291,9 @@
if (!command)
return;
- if (base::WriteFile(metrics_directory_.Append(metrics::kConsentFileName),
- "", 0) != 0) {
+ if (base::WriteFile(
+ shared_metrics_directory_.Append(metrics::kConsentFileName), "", 0) !=
+ 0) {
PLOG(ERROR) << "Could not create the consent file.";
command->Abort("metrics_error", "Could not create the consent file",
nullptr);
@@ -306,8 +310,8 @@
if (!command)
return;
- if (!base::DeleteFile(metrics_directory_.Append(metrics::kConsentFileName),
- false)) {
+ if (!base::DeleteFile(
+ shared_metrics_directory_.Append(metrics::kConsentFileName), false)) {
PLOG(ERROR) << "Could not delete the consent file.";
command->Abort("metrics_error", "Could not delete the consent file",
nullptr);
diff --git a/metricsd/metrics_collector.h b/metricsd/metrics_collector.h
index e080ac0..69747d0 100644
--- a/metricsd/metrics_collector.h
+++ b/metricsd/metrics_collector.h
@@ -48,7 +48,8 @@
void Init(bool testing,
MetricsLibraryInterface* metrics_lib,
const std::string& diskstats_path,
- const base::FilePath& metrics_directory);
+ const base::FilePath& private_metrics_directory,
+ const base::FilePath& shared_metrics_directory);
// Initializes DBus and MessageLoop variables before running the MessageLoop.
int OnInit() override;
@@ -225,8 +226,8 @@
// Test mode.
bool testing_;
- // Root of the configuration files to use.
- base::FilePath metrics_directory_;
+ // Publicly readable metrics directory.
+ base::FilePath shared_metrics_directory_;
// The metrics library handle.
MetricsLibraryInterface* metrics_lib_;
diff --git a/metricsd/metrics_collector_main.cc b/metricsd/metrics_collector_main.cc
index 117426e..d7aaaf5 100644
--- a/metricsd/metrics_collector_main.cc
+++ b/metricsd/metrics_collector_main.cc
@@ -51,9 +51,13 @@
int main(int argc, char** argv) {
DEFINE_bool(foreground, false, "Don't daemonize");
- DEFINE_string(metrics_directory,
- metrics::kMetricsDirectory,
- "Root of the configuration files (testing only)");
+ DEFINE_string(private_directory, metrics::kMetricsCollectorDirectory,
+ "Path to the private directory used by metrics_collector "
+ "(testing only)");
+ DEFINE_string(shared_directory, metrics::kSharedMetricsDirectory,
+ "Path to the shared metrics directory, used by "
+ "metrics_collector, metricsd and all metrics clients "
+ "(testing only)");
DEFINE_bool(logtostderr, false, "Log to standard error");
DEFINE_bool(logtosyslog, false, "Log to syslog");
@@ -86,7 +90,8 @@
daemon.Init(false,
&metrics_lib,
MetricsMainDiskStatsPath(),
- base::FilePath(FLAGS_metrics_directory));
+ base::FilePath(FLAGS_private_directory),
+ base::FilePath(FLAGS_shared_directory));
daemon.Run();
}
diff --git a/metricsd/metrics_collector_test.cc b/metricsd/metrics_collector_test.cc
index a0e7087..956e56b 100644
--- a/metricsd/metrics_collector_test.cc
+++ b/metricsd/metrics_collector_test.cc
@@ -46,9 +46,13 @@
brillo::FlagHelper::Init(0, nullptr, "");
EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
- chromeos_metrics::PersistentInteger::SetMetricsDirectory(
- temp_dir_.path().value());
- daemon_.Init(true, &metrics_lib_, "", temp_dir_.path());
+ base::FilePath private_dir = temp_dir_.path().Append("private");
+ base::FilePath shared_dir = temp_dir_.path().Append("shared");
+
+ EXPECT_TRUE(base::CreateDirectory(private_dir));
+ EXPECT_TRUE(base::CreateDirectory(shared_dir));
+
+ daemon_.Init(true, &metrics_lib_, "", private_dir, shared_dir);
}
// Adds a metrics library mock expectation that the specified metric
diff --git a/metricsd/metrics_library.cc b/metricsd/metrics_library.cc
index 735d39f..686c926 100644
--- a/metricsd/metrics_library.cc
+++ b/metricsd/metrics_library.cc
@@ -134,7 +134,7 @@
}
void MetricsLibrary::Init() {
- base::FilePath dir = base::FilePath(metrics::kMetricsDirectory);
+ base::FilePath dir = base::FilePath(metrics::kSharedMetricsDirectory);
uma_events_file_ = dir.Append(metrics::kMetricsEventsFileName);
consent_file_ = dir.Append(metrics::kConsentFileName);
cached_enabled_ = false;
diff --git a/metricsd/metricsd.rc b/metricsd/metricsd.rc
index b5e7b82..359d0d1 100644
--- a/metricsd/metricsd.rc
+++ b/metricsd/metricsd.rc
@@ -1,5 +1,7 @@
on post-fs-data
mkdir /data/misc/metrics 0770 system system
+ mkdir /data/misc/metricsd 0700 system system
+ mkdir /data/misc/metrics_collector 0700 system system
service metricsd /system/bin/metricsd --foreground --logtosyslog
class late_start
diff --git a/metricsd/metricsd_main.cc b/metricsd/metricsd_main.cc
index ab71e6b..eee8a94 100644
--- a/metricsd/metricsd_main.cc
+++ b/metricsd/metricsd_main.cc
@@ -43,9 +43,13 @@
DEFINE_string(server,
metrics::kMetricsServer,
"Server to upload the metrics to. (needs -uploader)");
- DEFINE_string(metrics_directory,
- metrics::kMetricsDirectory,
- "Root of the configuration files (testing only)");
+ DEFINE_string(private_directory, metrics::kMetricsdDirectory,
+ "Path to the private directory used by metricsd "
+ "(testing only)");
+ DEFINE_string(shared_directory, metrics::kSharedMetricsDirectory,
+ "Path to the shared metrics directory, used by "
+ "metrics_collector, metricsd and all metrics clients "
+ "(testing only)");
DEFINE_bool(logtostderr, false, "Log to standard error");
DEFINE_bool(logtosyslog, false, "Log to syslog");
@@ -72,9 +76,10 @@
return errno;
}
- UploadService service(FLAGS_server,
- base::TimeDelta::FromSeconds(FLAGS_upload_interval_secs),
- base::FilePath(FLAGS_metrics_directory));
+ UploadService service(
+ FLAGS_server, base::TimeDelta::FromSeconds(FLAGS_upload_interval_secs),
+ base::FilePath(FLAGS_private_directory),
+ base::FilePath(FLAGS_shared_directory));
service.Run();
}
diff --git a/metricsd/persistent_integer.cc b/metricsd/persistent_integer.cc
index ddc4b50..7fe355e 100644
--- a/metricsd/persistent_integer.cc
+++ b/metricsd/persistent_integer.cc
@@ -23,19 +23,15 @@
#include "constants.h"
-
namespace chromeos_metrics {
-// Static class member instantiation.
-std::string PersistentInteger::metrics_directory_ = metrics::kMetricsDirectory;
-
-PersistentInteger::PersistentInteger(const std::string& name) :
- value_(0),
+PersistentInteger::PersistentInteger(const std::string& name,
+ const base::FilePath& directory)
+ : value_(0),
version_(kVersion),
name_(name),
- synced_(false) {
- backing_file_name_ = metrics_directory_ + name_;
-}
+ backing_file_path_(directory.Append(name_)),
+ synced_(false) {}
PersistentInteger::~PersistentInteger() {}
@@ -62,23 +58,25 @@
}
void PersistentInteger::Write() {
- int fd = HANDLE_EINTR(open(backing_file_name_.c_str(),
+ int fd = HANDLE_EINTR(open(backing_file_path_.value().c_str(),
O_WRONLY | O_CREAT | O_TRUNC,
S_IWUSR | S_IRUSR | S_IRGRP | S_IROTH));
- PCHECK(fd >= 0) << "cannot open " << backing_file_name_ << " for writing";
+ PCHECK(fd >= 0) << "cannot open " << backing_file_path_.value()
+ << " for writing";
PCHECK((HANDLE_EINTR(write(fd, &version_, sizeof(version_))) ==
sizeof(version_)) &&
(HANDLE_EINTR(write(fd, &value_, sizeof(value_))) ==
sizeof(value_)))
- << "cannot write to " << backing_file_name_;
+ << "cannot write to " << backing_file_path_.value();
close(fd);
synced_ = true;
}
bool PersistentInteger::Read() {
- int fd = HANDLE_EINTR(open(backing_file_name_.c_str(), O_RDONLY));
+ int fd = HANDLE_EINTR(open(backing_file_path_.value().c_str(), O_RDONLY));
if (fd < 0) {
- PLOG(WARNING) << "cannot open " << backing_file_name_ << " for reading";
+ PLOG(WARNING) << "cannot open " << backing_file_path_.value()
+ << " for reading";
return false;
}
int32_t version;
@@ -95,9 +93,4 @@
return read_succeeded;
}
-void PersistentInteger::SetMetricsDirectory(const std::string& directory) {
- metrics_directory_ = directory;
-}
-
-
} // namespace chromeos_metrics
diff --git a/metricsd/persistent_integer.h b/metricsd/persistent_integer.h
index ecef3d1..96d9fc0 100644
--- a/metricsd/persistent_integer.h
+++ b/metricsd/persistent_integer.h
@@ -21,6 +21,8 @@
#include <string>
+#include <base/files/file_path.h>
+
namespace chromeos_metrics {
// PersistentIntegers is a named 64-bit integer value backed by a file.
@@ -29,7 +31,7 @@
class PersistentInteger {
public:
- explicit PersistentInteger(const std::string& name);
+ PersistentInteger(const std::string& name, const base::FilePath& directory);
// Virtual only because of mock.
virtual ~PersistentInteger();
@@ -50,10 +52,6 @@
// Virtual only because of mock.
virtual void Add(int64_t x);
- // Sets the directory path for all persistent integers.
- // This is used in unittests to change where the counters are stored.
- static void SetMetricsDirectory(const std::string& directory);
-
private:
static const int kVersion = 1001;
@@ -68,8 +66,7 @@
int64_t value_;
int32_t version_;
std::string name_;
- std::string backing_file_name_;
- static std::string metrics_directory_;
+ base::FilePath backing_file_path_;
bool synced_;
};
diff --git a/metricsd/persistent_integer_mock.h b/metricsd/persistent_integer_mock.h
index acc5389..0be54d4 100644
--- a/metricsd/persistent_integer_mock.h
+++ b/metricsd/persistent_integer_mock.h
@@ -27,9 +27,10 @@
class PersistentIntegerMock : public PersistentInteger {
public:
- explicit PersistentIntegerMock(const std::string& name)
- : PersistentInteger(name) {}
- MOCK_METHOD1(Add, void(int64_t count));
+ explicit PersistentIntegerMock(const std::string& name,
+ const base::FilePath& directory)
+ : PersistentInteger(name, directory) {}
+ MOCK_METHOD1(Add, void(int64_t count));
};
} // namespace chromeos_metrics
diff --git a/metricsd/persistent_integer_test.cc b/metricsd/persistent_integer_test.cc
index 5e2067f..55d6cbc 100644
--- a/metricsd/persistent_integer_test.cc
+++ b/metricsd/persistent_integer_test.cc
@@ -24,7 +24,6 @@
#include "persistent_integer.h"
const char kBackingFileName[] = "1.pibakf";
-const char kBackingFilePattern[] = "*.pibakf";
using chromeos_metrics::PersistentInteger;
@@ -32,28 +31,15 @@
void SetUp() override {
// Set testing mode.
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
- chromeos_metrics::PersistentInteger::SetMetricsDirectory(
- temp_dir_.path().value());
}
- void TearDown() override {
- // Remove backing files. The convention is that they all end in ".pibakf".
- base::FileEnumerator f_enum(base::FilePath("."),
- false,
- base::FileEnumerator::FILES,
- FILE_PATH_LITERAL(kBackingFilePattern));
- for (base::FilePath name = f_enum.Next();
- !name.empty();
- name = f_enum.Next()) {
- base::DeleteFile(name, false);
- }
- }
-
+ protected:
base::ScopedTempDir temp_dir_;
};
TEST_F(PersistentIntegerTest, BasicChecks) {
- scoped_ptr<PersistentInteger> pi(new PersistentInteger(kBackingFileName));
+ scoped_ptr<PersistentInteger> pi(
+ new PersistentInteger(kBackingFileName, temp_dir_.path()));
// Test initialization.
EXPECT_EQ(0, pi->Get());
@@ -65,7 +51,7 @@
EXPECT_EQ(5, pi->Get());
// Test persistence.
- pi.reset(new PersistentInteger(kBackingFileName));
+ pi.reset(new PersistentInteger(kBackingFileName, temp_dir_.path()));
EXPECT_EQ(5, pi->Get());
// Test GetAndClear.
@@ -73,6 +59,6 @@
EXPECT_EQ(pi->Get(), 0);
// Another persistence test.
- pi.reset(new PersistentInteger(kBackingFileName));
+ pi.reset(new PersistentInteger(kBackingFileName, temp_dir_.path()));
EXPECT_EQ(0, pi->Get());
}
diff --git a/metricsd/uploader/system_profile_cache.cc b/metricsd/uploader/system_profile_cache.cc
index 8928a0d..70f6afd 100644
--- a/metricsd/uploader/system_profile_cache.cc
+++ b/metricsd/uploader/system_profile_cache.cc
@@ -55,11 +55,10 @@
SystemProfileCache::SystemProfileCache()
: initialized_(false),
- testing_(false),
- metrics_directory_(metrics::kMetricsDirectory),
- session_id_(new chromeos_metrics::PersistentInteger(
- kPersistentSessionIdFilename)) {
-}
+ testing_(false),
+ metrics_directory_(metrics::kMetricsdDirectory),
+ session_id_(new chromeos_metrics::PersistentInteger(
+ kPersistentSessionIdFilename, metrics_directory_)) {}
SystemProfileCache::SystemProfileCache(bool testing,
const base::FilePath& metrics_directory)
@@ -67,8 +66,7 @@
testing_(testing),
metrics_directory_(metrics_directory),
session_id_(new chromeos_metrics::PersistentInteger(
- kPersistentSessionIdFilename)) {
-}
+ kPersistentSessionIdFilename, metrics_directory)) {}
bool SystemProfileCache::Initialize() {
CHECK(!initialized_)
diff --git a/metricsd/uploader/upload_service.cc b/metricsd/uploader/upload_service.cc
index ca5024e..3e0c503 100644
--- a/metricsd/uploader/upload_service.cc
+++ b/metricsd/uploader/upload_service.cc
@@ -43,14 +43,17 @@
UploadService::UploadService(const std::string& server,
const base::TimeDelta& upload_interval,
- const base::FilePath& metrics_directory)
+ const base::FilePath& private_metrics_directory,
+ const base::FilePath& shared_metrics_directory)
: histogram_snapshot_manager_(this),
sender_(new HttpSender(server)),
- failed_upload_count_(metrics::kFailedUploadCountName),
+ failed_upload_count_(metrics::kFailedUploadCountName,
+ private_metrics_directory),
upload_interval_(upload_interval) {
- metrics_file_ = metrics_directory.Append(metrics::kMetricsEventsFileName);
- staged_log_path_ = metrics_directory.Append(metrics::kStagedLogName);
- consent_file_ = metrics_directory.Append(metrics::kConsentFileName);
+ metrics_file_ =
+ shared_metrics_directory.Append(metrics::kMetricsEventsFileName);
+ staged_log_path_ = private_metrics_directory.Append(metrics::kStagedLogName);
+ consent_file_ = shared_metrics_directory.Append(metrics::kConsentFileName);
}
int UploadService::OnInit() {
@@ -265,4 +268,3 @@
bool UploadService::AreMetricsEnabled() {
return base::PathExists(consent_file_);
}
-
diff --git a/metricsd/uploader/upload_service.h b/metricsd/uploader/upload_service.h
index 7faf357..eed0d9d 100644
--- a/metricsd/uploader/upload_service.h
+++ b/metricsd/uploader/upload_service.h
@@ -71,7 +71,8 @@
public:
UploadService(const std::string& server,
const base::TimeDelta& upload_interval,
- const base::FilePath& metrics_directory);
+ const base::FilePath& private_metrics_directory,
+ const base::FilePath& shared_metrics_directory);
// Initializes the upload service.
int OnInit();
@@ -162,7 +163,7 @@
scoped_ptr<Sender> sender_;
chromeos_metrics::PersistentInteger failed_upload_count_;
scoped_ptr<MetricsLog> current_log_;
-
+
base::TimeDelta upload_interval_;
base::FilePath consent_file_;
diff --git a/metricsd/uploader/upload_service_test.cc b/metricsd/uploader/upload_service_test.cc
index 24e3127..9fc5e71 100644
--- a/metricsd/uploader/upload_service_test.cc
+++ b/metricsd/uploader/upload_service_test.cc
@@ -39,13 +39,18 @@
protected:
virtual void SetUp() {
CHECK(dir_.CreateUniqueTempDir());
- chromeos_metrics::PersistentInteger::SetMetricsDirectory(
- dir_.path().value());
- metrics_lib_.InitForTest(dir_.path());
- ASSERT_EQ(0, base::WriteFile(
- dir_.path().Append(metrics::kConsentFileName), "", 0));
- upload_service_.reset(new UploadService("", base::TimeDelta(),
- dir_.path()));
+
+ base::FilePath private_dir = dir_.path().Append("private");
+ base::FilePath shared_dir = dir_.path().Append("shared");
+
+ EXPECT_TRUE(base::CreateDirectory(private_dir));
+ EXPECT_TRUE(base::CreateDirectory(shared_dir));
+
+ metrics_lib_.InitForTest(shared_dir);
+ ASSERT_EQ(0, base::WriteFile(shared_dir.Append(metrics::kConsentFileName),
+ "", 0));
+ upload_service_.reset(
+ new UploadService("", base::TimeDelta(), private_dir, shared_dir));
upload_service_->sender_.reset(new SenderMock);
upload_service_->InitForTest(new MockSystemProfileSetter);
@@ -160,7 +165,7 @@
}
TEST_F(UploadServiceTest, LogEmptyByDefault) {
- UploadService upload_service("", base::TimeDelta(), dir_.path());
+ UploadService upload_service("", base::TimeDelta(), dir_.path(), dir_.path());
// current_log_ should be initialized later as it needs AtExitManager to exit
// in order to gather system information from SysInfo.
@@ -196,7 +201,6 @@
metrics::MetricSample::HistogramSample("foo", 10, 0, 42, 10);
upload_service_->AddSample(*histogram.get());
-
scoped_ptr<metrics::MetricSample> histogram2 =
metrics::MetricSample::HistogramSample("foo", 11, 0, 42, 10);
upload_service_->AddSample(*histogram2.get());