bootstat: Track record inconsistencies w/ a debug metric, bootstat_mtime_matches_content.

Fixed a file descriptor leak while I was in here.

Bug: 27550578
Change-Id: I8e252e4f5bb3c4e2ae96a1560fbb32ae636722a0
(cherry picked from commit 6f28299d0d)

bootstat/Android.mk

@@ -21,6 +21,7 @@ bootstat_c_includes := external/gtest/include
 bootstat_lib_src_files := \
         boot_event_record_store.cpp \
         event_log_list_builder.cpp \
+        histogram_logger.cpp \
         uptime_parser.cpp \
 
 bootstat_src_files := \
@@ -41,17 +42,13 @@ bootstat_cflags := \
         -Wextra \
         -Werror \
 
-bootstat_cppflags := \
-        -Wno-non-virtual-dtor \
-
-bootstat_debug_cflags := \
-        $(bootstat_cflags) \
-        -UNDEBUG \
-
 # 524291 corresponds to sysui_histogram, from
 # frameworks/base/core/java/com/android/internal/logging/EventLogTags.logtags
 bootstat_cflags += -DHISTOGRAM_LOG_TAG=524291
 
+bootstat_debug_cflags := \
+        $(bootstat_cflags) \
+        -UNDEBUG \
 
 # bootstat static library
 # -----------------------------------------------------------------------------
@@ -60,7 +57,6 @@ include $(CLEAR_VARS)
 
 LOCAL_MODULE := libbootstat
 LOCAL_CFLAGS := $(bootstat_cflags)
-LOCAL_CPPFLAGS := $(bootstat_cppflags)
 LOCAL_C_INCLUDES := $(bootstat_c_includes)
 LOCAL_SHARED_LIBRARIES := $(bootstat_shared_libs)
 LOCAL_SRC_FILES := $(bootstat_lib_src_files)
@@ -76,7 +72,6 @@ include $(CLEAR_VARS)
 
 LOCAL_MODULE := libbootstat_debug
 LOCAL_CFLAGS := $(bootstat_cflags)
-LOCAL_CPPFLAGS := $(bootstat_debug_cppflags)
 LOCAL_C_INCLUDES := $(bootstat_c_includes)
 LOCAL_SHARED_LIBRARIES := $(bootstat_shared_libs)
 LOCAL_SRC_FILES := $(bootstat_lib_src_files)
@@ -92,7 +87,6 @@ include $(CLEAR_VARS)
 
 LOCAL_MODULE := libbootstat_host_debug
 LOCAL_CFLAGS := $(bootstat_debug_cflags)
-LOCAL_CPPFLAGS := $(bootstat_cppflags)
 LOCAL_C_INCLUDES := $(bootstat_c_includes)
 LOCAL_SHARED_LIBRARIES := $(bootstat_shared_libs)
 LOCAL_SRC_FILES := $(bootstat_lib_src_files)
@@ -108,7 +102,6 @@ include $(CLEAR_VARS)
 
 LOCAL_MODULE := bootstat
 LOCAL_CFLAGS := $(bootstat_cflags)
-LOCAL_CPPFLAGS := $(bootstat_cppflags)
 LOCAL_C_INCLUDES := $(bootstat_c_includes)
 LOCAL_SHARED_LIBRARIES := $(bootstat_shared_libs)
 LOCAL_STATIC_LIBRARIES := libbootstat
@@ -126,7 +119,6 @@ include $(CLEAR_VARS)
 
 LOCAL_MODULE := bootstat_tests
 LOCAL_CFLAGS := $(bootstat_tests_cflags)
-LOCAL_CPPFLAGS := $(bootstat_cppflags)
 LOCAL_SHARED_LIBRARIES := $(bootstat_shared_libs)
 LOCAL_STATIC_LIBRARIES := libbootstat_debug libgmock
 LOCAL_SRC_FILES := $(bootstat_test_src_files)
@@ -142,7 +134,6 @@ include $(CLEAR_VARS)
 
 LOCAL_MODULE := bootstat_tests
 LOCAL_CFLAGS := $(bootstat_tests_cflags)
-LOCAL_CPPFLAGS := $(bootstat_cppflags)
 LOCAL_SHARED_LIBRARIES := $(bootstat_shared_libs)
 LOCAL_STATIC_LIBRARIES := libbootstat_host_debug libgmock_host
 LOCAL_SRC_FILES := $(bootstat_test_src_files)

bootstat/boot_event_record_store.cpp

@@ -21,9 +21,11 @@
 #include <sys/stat.h>
 #include <utime.h>
 #include <cstdlib>
+#include <string>
 #include <utility>
 #include <android-base/file.h>
 #include <android-base/logging.h>
+#include "histogram_logger.h"
 #include "uptime_parser.h"
 
 namespace {
@@ -42,6 +44,20 @@ bool ParseRecordEventTime(const std::string& path, int32_t* uptime) {
   }
 
   *uptime = file_stat.st_mtime;
+
+  // The following code (till function exit) is a debug test to ensure the
+  // validity of the file mtime value, i.e., to check that the record file
+  // mtime values are not changed once set.
+  // TODO(jhawkins): Remove this code.
+  std::string content;
+  if (!android::base::ReadFileToString(path, &content)) {
+    PLOG(ERROR) << "Failed to read " << path;
+    return false;
+  }
+
+  int32_t value = std::stoi(content);
+  bootstat::LogHistogram("bootstat_mtime_matches_content", value == *uptime);
+
   return true;
 }
 
@@ -61,8 +77,20 @@ void BootEventRecordStore::AddBootEvent(const std::string& event) {
 void BootEventRecordStore::AddBootEventWithValue(
     const std::string& event, int32_t value) {
   std::string record_path = GetBootEventPath(event);
-  if (creat(record_path.c_str(), S_IRUSR | S_IWUSR) == -1) {
+  int record_fd = creat(record_path.c_str(), S_IRUSR | S_IWUSR);
+  if (record_fd == -1) {
     PLOG(ERROR) << "Failed to create " << record_path;
+    return;
+  }
+
+  // Writing the value as content in the record file is a debug measure to
+  // ensure the validity of the file mtime value, i.e., to check that the record
+  // file mtime values are not changed once set.
+  // TODO(jhawkins): Remove this block.
+  if (!android::base::WriteStringToFd(std::to_string(value), record_fd)) {
+    PLOG(ERROR) << "Failed to write value to " << record_path;
+    close(record_fd);
+    return;
   }
 
   // Fill out the stat structure for |record_path| in order to get the atime to
@@ -70,6 +98,8 @@ void BootEventRecordStore::AddBootEventWithValue(
   struct stat file_stat;
   if (stat(record_path.c_str(), &file_stat) == -1) {
     PLOG(ERROR) << "Failed to read " << record_path;
+    close(record_fd);
+    return;
   }
 
   // Set the |modtime| of the file to store the value of the boot event while
@@ -77,7 +107,11 @@ void BootEventRecordStore::AddBootEventWithValue(
   struct utimbuf times = {/* actime */ file_stat.st_atime, /* modtime */ value};
   if (utime(record_path.c_str(), &times) == -1) {
     PLOG(ERROR) << "Failed to set mtime for " << record_path;
+    close(record_fd);
+    return;
   }
+
+  close(record_fd);
 }
 
 bool BootEventRecordStore::GetBootEvent(

bootstat/bootstat.cpp

@@ -32,26 +32,11 @@
 #include <log/log.h>
 #include "boot_event_record_store.h"
 #include "event_log_list_builder.h"
+#include "histogram_logger.h"
 #include "uptime_parser.h"
 
 namespace {
 
-// Builds an EventLog buffer named |event| containing |data| and writes
-// the log into the Tron histogram logs.
-void LogBootEvent(const std::string& event, int32_t data) {
-  LOG(INFO) << "Logging boot metric: " << event << " " << data;
-
-  EventLogListBuilder log_builder;
-  log_builder.Append(event);
-  log_builder.Append(data);
-
-  std::unique_ptr<uint8_t[]> log;
-  size_t size;
-  log_builder.Release(&log, &size);
-
-  android_bWriteLog(HISTOGRAM_LOG_TAG, log.get(), size);
-}
-
 // Scans the boot event record store for record files and logs each boot event
 // via EventLog.
 void LogBootEvents() {
@@ -59,7 +44,7 @@ void LogBootEvents() {
 
   auto events = boot_event_store.GetAllBootEvents();
   for (auto i = events.cbegin(); i != events.cend(); ++i) {
-    LogBootEvent(i->first, i->second);
+    bootstat::LogHistogram(i->first, i->second);
   }
 }
 
@@ -202,10 +187,10 @@ void RecordFactoryReset() {
   static const char* factory_reset_current_time = "factory_reset_current_time";
   if (current_time_utc < 0) {
     // UMA does not display negative values in buckets, so convert to positive.
-    LogBootEvent(factory_reset_current_time, std::abs(current_time_utc));
+    bootstat::LogHistogram(factory_reset_current_time, std::abs(current_time_utc));
     return;
   } else {
-    LogBootEvent(factory_reset_current_time, current_time_utc);
+    bootstat::LogHistogram(factory_reset_current_time, current_time_utc);
   }
 
   // The factory_reset boot event does not exist after the device is reset, so
@@ -221,7 +206,7 @@ void RecordFactoryReset() {
   // Calculate and record the difference in time between now and the
   // factory_reset time.
   time_t factory_reset_utc = record.second;
-  LogBootEvent("factory_reset_record_value", factory_reset_utc);
+  bootstat::LogHistogram("factory_reset_record_value", factory_reset_utc);
   time_t time_since_factory_reset = difftime(current_time_utc,
                                              factory_reset_utc);
   boot_event_store.AddBootEventWithValue("time_since_factory_reset",

bootstat/histogram_logger.cpp

@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "histogram_logger.h"
+
+#include <cstdlib>
+#include <memory>
+#include <android-base/logging.h>
+#include <log/log.h>
+#include "event_log_list_builder.h"
+
+namespace bootstat {
+
+void LogHistogram(const std::string& event, int32_t data) {
+  LOG(INFO) << "Logging histogram: " << event << " " << data;
+
+  EventLogListBuilder log_builder;
+  log_builder.Append(event);
+  log_builder.Append(data);
+
+  std::unique_ptr<uint8_t[]> log;
+  size_t size;
+  log_builder.Release(&log, &size);
+
+  android_bWriteLog(HISTOGRAM_LOG_TAG, log.get(), size);
+}
+
+}  // namespace bootstat

bootstat/histogram_logger.h

@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <cstdint>
+#include <string>
+
+namespace bootstat {
+
+// Builds an EventLog buffer named |event| containing |data| and writes
+// the log into the Tron histogram logs.
+void LogHistogram(const std::string& event, int32_t data);
+
+}  // namespace bootstat