Create a bootstat library for external C and C++ programs
BUG=none
TEST=emerge-x86-generic bootstat
Review URL: http://codereview.chromium.org/3129002
diff --git a/Makefile b/Makefile
index 697875b..2066cdc 100644
--- a/Makefile
+++ b/Makefile
@@ -5,19 +5,42 @@
# Makefile for bootstat utilities
#
-OPT_FLAGS = -O
-CFLAGS += -Wall -Werror -fPIC -fno-exceptions $(OPT_FLAGS)
+# OPT_CFLAGS is here to allow adding options for manual, host-side
+# builds.
+OPT_CFLAGS =
+CFLAGS += -Wall -Werror -fPIC -fno-exceptions $(OPT_CFLAGS)
-OBJS = \
- bootstat.o \
- bootstat_log.o
+LIB_OBJS = bootstat_log.o
+COMMAND_OBJS = bootstat.o
+COMMAND_LDFLAGS = $(LDFLAGS)
+
+TEST_OBJS = log_unit_tests.o
+TEST_LDFLAGS = $(LDFLAGS) -lgtest
+
+OBJS = $(LIB_OBJS) $(TEST_OBJS) $(COMMAND_OBJS)
+
+LIB = libbootstat.a
COMMAND = bootstat
+TEST = log_unit_test
-all: $(COMMAND)
+%.o: %.c
+ $(CC) $(CFLAGS) -o $@ -c $<
-$(COMMAND): $(OBJS)
- $(CC) -o $@ $(LDFLAGS) $(OBJS)
+%.o: %.cc
+ $(CXX) $(CXXFLAGS) -o $@ -c $<
+
+all: $(COMMAND) $(LIB)
+tests: $(TEST)
+
+$(COMMAND): $(COMMAND_OBJS) $(LIB)
+ $(CC) -o $@ $(COMMAND_LDFLAGS) $^
+
+$(LIB): $(LIB_OBJS)
+ $(AR) rcs $@ $^
+
+$(TEST): $(TEST_OBJS) $(LIB)
+ $(CXX) $(TEST_LDFLAGS) -o $@ $^
clean:
- rm -f $(COMMAND) $(OBJS)
+ rm -f $(COMMAND) $(LIB) $(OBJS) $(TEST)
diff --git a/README b/README
index 6a3d0a0..787e5f0 100644
--- a/README
+++ b/README
@@ -15,13 +15,17 @@
not the boot partition), and associates the data with the passed
in <event-name>.
-==== Design details and API
+==== API specification
+The C and C++ API is defined in "bootstat.h". See that header for
+specification details.
+
+==== Design and implementation details
Uptime data are stored in a file named /tmp/uptime-<event-name>;
disk statistics are stored in a file named /tmp/disk-<event-name>.
-This file naming convention is a concession to existing code that
-depends on these files existing with these specific names, including
-the platform_BootPerf test in autotest, /etc/boot-complete.conf, and
-portions of Chrome.
+This convention is a concession to pre-existing code that depends on
+these files existing with these specific names, including the
+platform_BootPerf test in autotest, the boot-complete upstart job,
+and the Chrome code to report boot time on the login screen.
New code should treat the file names as an implementation detail,
not as the interface. You should not add new code that depends on
@@ -29,12 +33,14 @@
and/or library to provide access to the data you need.
==== Code conventions
-This is currently C code, because a) the code is intended for use
-as a library accessible to C and C++ code, and b) it's too small to
-make sense using C++ features. However, if the program grows, it
-may be appropriate to convert all or part to C++.
+This is currently C code, because a) the code is required for use
+as a library accessible to both C and C++ code, and b) it's too
+small to justify the boilerplate required to have separate C and
+C++ bindings. However, if the program grows, it may be appropriate
+to convert all or part to C++.
-To the extent that the code should be acceptable to a C++ compiler,
-it should also adhere to Google's C++ coding conventions. In areas
-where the code is C specific, use your best judgement (note that
-Google also has Objective-C coding conventions that may be useful).
+To the extent that the code can be acceptable both to C and C++
+compilers, it should also adhere to Google's C++ coding conventions.
+In areas where the code is C specific, use your best judgement (note
+that Google also has Objective-C coding conventions that may be
+useful).
diff --git a/bootstat.c b/bootstat.c
index 3a2f1f6..573f9ef 100644
--- a/bootstat.c
+++ b/bootstat.c
@@ -2,6 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+// Implementation of the 'bootstat' command, part of the Chromium OS
+// 'bootstat' facility. The command provides a command line wrapper
+// around the key functionality declared in "bootstat.h"
+
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -10,14 +14,14 @@
#include "bootstat.h"
-static void usage(char *cmd)
+static void usage(char* cmd)
{
fprintf(stderr, "usage: %s <event-name>\n", basename(strdup(cmd)));
exit(EXIT_FAILURE);
}
-int main(int argc, char *argv[])
+int main(int argc, char* argv[])
{
if (argc != 2)
usage(argv[0]);
diff --git a/bootstat.h b/bootstat.h
index d198061..d6b13d2 100644
--- a/bootstat.h
+++ b/bootstat.h
@@ -2,6 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+// Public API for the C and C++ bindings to the Chromium OS
+// 'bootstat' facility. The facility is a simple timestamp
+// mechanism to associate a named event with the time that it
+// occurred and with other relevant statistics.
+
#ifndef BOOTSTAT_H_
#define BOOTSTAT_H_
@@ -9,9 +14,29 @@
extern "C" {
#endif
-extern void bootstat_log(const char *);
+//
+// Length of the longest valid string naming an event, including the
+// terminating NUL character. Clients of bootstat_log() can use
+// this value for the size of buffers to hold event names; names
+// that exceed this buffer size will be truncated.
+//
+// This value is arbitrarily chosen, but see comments in
+// bootstat_log.c regarding implementation assumptions for this
+// value.
+//
+#define BOOTSTAT_MAX_EVENT_LEN 64
+
+// Log an event. Event names should be composed of characters drawn
+// from this subset of 7-bit ASCII: Letters (upper- or lower-case),
+// digits, dot ('.'), dash ('-'), and underscore ('_'). Case is
+// significant. Behavior in the presence of other characters is
+// unspecified - Caveat Emptor!
+//
+// Applications are responsible for establishing higher-level naming
+// conventions to prevent name collisions.
+extern void bootstat_log(const char* event_name);
#if defined(__cplusplus)
}
#endif
-#endif // BOOTSTAT_H_
+#endif // BOOTSTAT_H_
diff --git a/bootstat_log.c b/bootstat_log.c
index e82ef88..079c6f2 100644
--- a/bootstat_log.c
+++ b/bootstat_log.c
@@ -2,6 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+// Implementation of bootstat_log(), part of the Chromium OS 'bootstat'
+// facility.
+
+#include "bootstat.h"
+
#include <stdio.h>
#include <stddef.h>
@@ -10,54 +15,44 @@
#include <sys/fcntl.h>
#include <unistd.h>
-#include "bootstat.h"
-
//
// Paths to the statistics files we snapshot as part of the data to
// be logged.
//
-#define UPTIME "/proc/uptime"
+static const char kUptimeStatisticsFileName[] = "/proc/uptime";
-#if defined (__amd64__) || defined (__x86_64__)
-#define ROOTDEV "sda"
+#if defined (__amd64__) || defined (__x86_64__) || defined (__i386__)
+static const char kDiskStatisticsFileName[] = "/sys/block/sda/stat";
+#elif defined (__arm__)
+static const char kDiskStatisticsFileName[] = "/sys/block/mmcblk0/stat";
+#else
+#error "unknown processor type?"
#endif
-#if defined (__arm__)
-#define ROOTDEV "mmcblk0"
-#endif
-
-#ifdef ROOTDEV
-#define ROOTSTAT "/sys/block/" ROOTDEV "/stat"
-#endif
-
-//
-// Length of the longest valid string naming an event, not counting
-// the terminating NUL character. Arbitrarily chosen, but intended
-// to be somewhat shorter than any valid file or path name.
-//
-#define MAX_EVENT_LEN 63
//
// Maximum length of any pathname for storing event statistics.
-// Also arbitrarily chosen, but see the comment below about
-// truncation.
+// Arbitrarily chosen, but see the comment below about truncation.
//
#define MAX_STAT_PATH 128
//
// Output file creation mode: 0666, a.k.a. rw-rw-rw-.
//
-#define OFMODE (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)
+static const int kFileCreationMode =
+ (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
-static void append_logdata(const char *ifname,
- const char *ofprefix, const char *event_id)
+
+static void append_logdata(const char* input_path,
+ const char* output_name_prefix,
+ const char* event_name)
{
- char ofpath[MAX_STAT_PATH];
- char buffer[256];
- ssize_t nbytes;
- int ifd, ofd;
+ char output_path[MAX_STAT_PATH];
+ char buffer[256];
+ ssize_t num_read;
+ int ifd, ofd;
- ifd = open(ifname, O_RDONLY);
+ ifd = open(input_path, O_RDONLY);
if (ifd < 0) {
return;
}
@@ -65,25 +60,24 @@
//
// We don't want the file name "/tmp/uptime-..." truncated
// differently from the the name "/tmp/disk-...", so we truncate
- // event_id separately using the "%.*s" format.
+ // event_name separately using the "%.*s" format.
//
- // We expect that MAX_STAT_LEN is enough smaller than
- // MAX_STAT_PATH that ofpath will never be truncated. However,
- // to be safe, we also stuff our own terminating '\0', since
- // snprintf() won't put it there in the case of truncation.
+ // We expect that BOOTSTAT_MAX_EVENT_LEN is enough smaller than
+ // MAX_STAT_PATH that output_path will never be truncated.
//
- (void) snprintf(ofpath, sizeof (ofpath) - 1, "/tmp/%s-%.*s",
- ofprefix, MAX_EVENT_LEN, event_id);
- ofpath[sizeof (ofpath) - 1] = '\0';
- ofd = open(ofpath, O_WRONLY | O_APPEND | O_CREAT, OFMODE);
+ (void) snprintf(output_path, sizeof(output_path), "/tmp/%s-%.*s",
+ output_name_prefix,
+ BOOTSTAT_MAX_EVENT_LEN - 1, event_name);
+ ofd = open(output_path, O_WRONLY | O_APPEND | O_CREAT,
+ kFileCreationMode);
if (ofd < 0) {
(void) close(ifd);
return;
}
- while ((nbytes = read(ifd, buffer, sizeof (buffer))) > 0) {
- ssize_t wnbytes = write(ofd, buffer, nbytes);
- if (wnbytes != nbytes)
+ while ((num_read = read(ifd, buffer, sizeof(buffer))) > 0) {
+ ssize_t num_written = write(ofd, buffer, num_read);
+ if (num_written != num_read)
break;
}
(void) close(ofd);
@@ -91,10 +85,8 @@
}
-void bootstat_log(const char *event_id)
+void bootstat_log(const char* event_name)
{
- append_logdata(UPTIME, "uptime", event_id);
-#ifdef ROOTSTAT
- append_logdata(ROOTSTAT, "disk", event_id);
-#endif
+ append_logdata(kUptimeStatisticsFileName, "uptime", event_name);
+ append_logdata(kDiskStatisticsFileName, "disk", event_name);
}
diff --git a/log_unit_tests.cc b/log_unit_tests.cc
new file mode 100644
index 0000000..2d279ee
--- /dev/null
+++ b/log_unit_tests.cc
@@ -0,0 +1,60 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "bootstat.h"
+
+#include <errno.h>
+#include <unistd.h>
+
+#include <string>
+#include <iostream>
+
+#include <gtest/gtest.h>
+
+namespace {
+
+using std::string;
+
+static const char kMostVoluminousEventName[] =
+ // 16 32 48 64
+ "event-6789abcdef_123456789ABCDEF.123456789abcdef0123456789abcdef" // 64
+ "event-6789abcdef_123456789ABCDEF.123456789abcdef0123456789abcdef" // 128
+ "event-6789abcdef_123456789ABCDEF.123456789abcdef0123456789abcdef" // 191
+ "event-6789abcdef_123456789ABCDEF.123456789abcdef0123456789abcdef" // 256
+ ;
+
+static const string kUptimeFileNamePrefix("/tmp/uptime-");
+static const string kDiskStatFileNamePrefix("/tmp/disk-");
+
+static void TestEventFileAccess(const string& file_name) {
+ int rv = access(file_name.c_str(), R_OK | W_OK);
+ EXPECT_EQ(rv, 0) << "access to " << file_name
+ << " failed: " << strerror(errno);
+ (void) unlink(file_name.c_str());
+}
+
+static void TestEventByName(const string& event_name) {
+ bootstat_log(event_name.c_str());
+ string truncated_event(event_name.substr(0, BOOTSTAT_MAX_EVENT_LEN - 1));
+ TestEventFileAccess(kUptimeFileNamePrefix + truncated_event);
+ TestEventFileAccess(kDiskStatFileNamePrefix + truncated_event);
+}
+
+// Tests that name truncation of logged events works as advertised
+TEST(BoostatTest, EventNameTruncation) {
+ string very_long(kMostVoluminousEventName);
+
+ TestEventByName(very_long);
+ TestEventByName(very_long.substr(0, 1));
+ TestEventByName(very_long.substr(0, 16));
+ TestEventByName(very_long.substr(0, BOOTSTAT_MAX_EVENT_LEN - 1));
+ TestEventByName(very_long.substr(0, BOOTSTAT_MAX_EVENT_LEN));
+}
+
+} // namespace
+
+int main(int argc, char** argv) {
+ testing::InitGoogleTest(&argc, argv);
+ return RUN_ALL_TESTS();
+}
