Clean up reading and writing in init.

This isn't particularly useful in and of itself, but it does introduce the
first (trivial) unit test, improves the documentation (including details
about how to debug init crashes), and made me aware of how unpleasant the
existing parser is.

I also fixed a bug in passing --- unless you thought the "peboot" and "pm"
commands were features...

Bug: 19217569
Change-Id: I6ab76129a543ce3ed3dab52ef2c638009874c3de

11 files changed, 184 insertions, 149 deletions

diff --git a/init/Android.mk b/init/Android.mk
index bf8dea5..7f3788a 100644
--- a/init/Android.mk
+++ b/init/Android.mk
@@ -1,48 +1,55 @@
 # Copyright 2005 The Android Open Source Project
 
 LOCAL_PATH:= $(call my-dir)
-include $(CLEAR_VARS)
 
 # --
 
 ifeq ($(strip $(INIT_BOOTCHART)),true)
-LOCAL_CPPFLAGS  += -DBOOTCHART=1
+init_options += -DBOOTCHART=1
 else
-LOCAL_CPPFLAGS  += -DBOOTCHART=0
+init_options  += -DBOOTCHART=0
 endif
 
 ifneq (,$(filter userdebug eng,$(TARGET_BUILD_VARIANT)))
-LOCAL_CPPFLAGS += -DALLOW_LOCAL_PROP_OVERRIDE=1 -DALLOW_DISABLE_SELINUX=1
+init_options += -DALLOW_LOCAL_PROP_OVERRIDE=1 -DALLOW_DISABLE_SELINUX=1
 else
-LOCAL_CPPFLAGS += -DALLOW_LOCAL_PROP_OVERRIDE=0 -DALLOW_DISABLE_SELINUX=0
+init_options += -DALLOW_LOCAL_PROP_OVERRIDE=0 -DALLOW_DISABLE_SELINUX=0
 endif
 
-LOCAL_CPPFLAGS += -DLOG_UEVENTS=0
+init_options += -DLOG_UEVENTS=0
+
+init_cflags += \
+    $(init_options) \
+    -Wall -Wextra \
+    -Wno-unused-parameter \
+    -Werror \
 
 # --
 
+include $(CLEAR_VARS)
+LOCAL_CPPFLAGS := $(init_cflags)
+LOCAL_SRC_FILES:= \
+    init_parser.cpp \
+    parser.cpp \
+    util.cpp \
+
+LOCAL_MODULE := libinit
+include $(BUILD_STATIC_LIBRARY)
+
+include $(CLEAR_VARS)
+LOCAL_CPPFLAGS := $(init_cflags)
 LOCAL_SRC_FILES:= \
     bootchart.cpp \
     builtins.cpp \
     devices.cpp \
     init.cpp \
-    init_parser.cpp \
     keychords.cpp \
-    parser.cpp \
     property_service.cpp \
     signal_handler.cpp \
     ueventd.cpp \
     ueventd_parser.cpp \
-    util.cpp \
     watchdogd.cpp \
 
-#LOCAL_CLANG := true
-
-LOCAL_CPPFLAGS += \
-    -Wall -Wextra \
-    -Werror -Wno-error=deprecated-declarations \
-    -Wno-unused-parameter \
-
 LOCAL_MODULE:= init
 
 LOCAL_FORCE_STATIC_EXECUTABLE := true
@@ -50,14 +57,16 @@ LOCAL_MODULE_PATH := $(TARGET_ROOT_OUT)
 LOCAL_UNSTRIPPED_PATH := $(TARGET_ROOT_OUT_UNSTRIPPED)
 
 LOCAL_STATIC_LIBRARIES := \
-	libfs_mgr \
-	liblogwrap \
-	libcutils \
-	liblog \
-	libc \
-	libselinux \
-	libmincrypt \
-	libext4_utils_static
+    libinit \
+    libfs_mgr \
+    liblogwrap \
+    libcutils \
+    libutils \
+    liblog \
+    libc \
+    libselinux \
+    libmincrypt \
+    libext4_utils_static
 
 # Create symlinks
 LOCAL_POST_INSTALL_CMD := $(hide) mkdir -p $(TARGET_ROOT_OUT)/sbin; \
@@ -65,3 +74,18 @@ LOCAL_POST_INSTALL_CMD := $(hide) mkdir -p $(TARGET_ROOT_OUT)/sbin; \
     ln -sf ../init $(TARGET_ROOT_OUT)/sbin/watchdogd
 
 include $(BUILD_EXECUTABLE)
+
+
+
+
+include $(CLEAR_VARS)
+LOCAL_MODULE := init_tests
+LOCAL_SRC_FILES := \
+    util_test.cpp \
+
+LOCAL_SHARED_LIBRARIES += \
+    libcutils \
+    libutils \
+
+LOCAL_STATIC_LIBRARIES := libinit
+include $(BUILD_NATIVE_TEST)
diff --git a/init/builtins.cpp b/init/builtins.cpp
index 9ead340..31c6a99 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -56,42 +56,15 @@ int add_environment(const char *name, const char *value);
 // System call provided by bionic but not in any header file.
 extern "C" int init_module(void *, unsigned long, const char *);
 
-static int write_file(const char *path, const char *value)
-{
-    int fd, ret, len;
-
-    fd = open(path, O_WRONLY|O_CREAT|O_NOFOLLOW|O_CLOEXEC, 0600);
-
-    if (fd < 0)
-        return -errno;
-
-    len = strlen(value);
-
-    ret = TEMP_FAILURE_RETRY(write(fd, value, len));
-
-    close(fd);
-    if (ret < 0) {
-        return -errno;
-    } else {
-        return 0;
-    }
-}
-
 static int insmod(const char *filename, char *options)
 {
-    void *module;
-    unsigned size;
-    int ret;
-
-    module = read_file(filename, &size);
-    if (!module)
+    std::string module;
+    if (!read_file(filename, &module)) {
         return -1;
+    }
 
-    ret = init_module(module, size, options);
-
-    free(module);
-
-    return ret;
+    // TODO: use finit_module for >= 3.8 kernels.
+    return init_module(&module[0], module.size(), options);
 }
 
 static int setkey(struct kbentry *kbe)
@@ -743,15 +716,13 @@ int do_write(int nargs, char **args)
 {
     const char *path = args[1];
     const char *value = args[2];
-    char prop_val[PROP_VALUE_MAX];
-    int ret;
 
-    ret = expand_props(prop_val, value, sizeof(prop_val));
-    if (ret) {
+    char expanded_value[PROP_VALUE_MAX];
+    if (expand_props(expanded_value, value, sizeof(expanded_value))) {
         ERROR("cannot expand '%s' while writing to '%s'\n", value, path);
         return -EINVAL;
     }
-    return write_file(path, prop_val);
+    return write_file(path, expanded_value);
 }
 
 int do_copy(int nargs, char **args)
diff --git a/init/devices.cpp b/init/devices.cpp
index b55933c..9275439 100644
--- a/init/devices.cpp
+++ b/init/devices.cpp
@@ -48,8 +48,6 @@
 #include "util.h"
 #include "log.h"
 
-#define UNUSED __attribute__((__unused__))
-
 #define SYSFS_PREFIX    "/sys"
 #define FIRMWARE_DIR1   "/etc/firmware"
 #define FIRMWARE_DIR2   "/vendor/firmware"
@@ -231,7 +229,7 @@ static mode_t get_device_perm(const char *path, const char **links,
 }
 
 static void make_device(const char *path,
-                        const char *upath UNUSED,
+                        const char */*upath*/,
                         int block, int major, int minor,
                         const char **links)
 {
@@ -652,11 +650,6 @@ static void mkdir_recursive_for_devpath(const char *devpath)
     mkdir_recursive(dir, 0755);
 }
 
-static inline void __attribute__((__deprecated__)) kernel_logger()
-{
-    INFO("kernel logger is deprecated\n");
-}
-
 static void handle_generic_device_event(struct uevent *uevent)
 {
     const char *base;
@@ -743,7 +736,7 @@ static void handle_generic_device_event(struct uevent *uevent)
          make_dir(base, 0755);
      } else if(!strncmp(uevent->subsystem, "misc", 4) &&
                  !strncmp(name, "log_", 4)) {
-         kernel_logger();
+         INFO("kernel logger is deprecated\n");
          base = "/dev/log/";
          make_dir(base, 0755);
          name += 4;
diff --git a/init/init.cpp b/init/init.cpp
index da3fe96..3e3be2e 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -940,7 +940,7 @@ int selinux_reload_policy(void)
     return 0;
 }
 
-static int audit_callback(void *data, security_class_t cls __attribute__((unused)), char *buf, size_t len)
+static int audit_callback(void *data, security_class_t /*cls*/, char *buf, size_t len)
 {
     snprintf(buf, len, "property=%s", !data ? "NULL" : (char *)data);
     return 0;
@@ -1058,7 +1058,6 @@ int main(int argc, char **argv)
     INFO("property init\n");
     property_load_boot_defaults();
 
-    INFO("reading config file\n");
     init_parse_config_file("/init.rc");
 
     action_for_each_trigger("early-init", action_add_queue_tail);
@@ -1088,7 +1087,6 @@ int main(int argc, char **argv)
     /* run all property triggers based on current state of the properties */
     queue_builtin_action(queue_property_triggers_action, "queue_property_triggers");
 
-
     if (BOOTCHART) {
         queue_builtin_action(bootchart_init_action, "bootchart_init");
     }
diff --git a/init/init_parser.cpp b/init/init_parser.cpp
index facb0cf..65fc1a6 100644
--- a/init/init_parser.cpp
+++ b/init/init_parser.cpp
@@ -116,7 +116,7 @@ static int lookup_keyword(const char *s)
 {
     switch (*s++) {
     case 'c':
-    if (!strcmp(s, "opy")) return K_copy;
+        if (!strcmp(s, "opy")) return K_copy;
         if (!strcmp(s, "apability")) return K_capability;
         if (!strcmp(s, "hdir")) return K_chdir;
         if (!strcmp(s, "hroot")) return K_chroot;
@@ -171,6 +171,7 @@ static int lookup_keyword(const char *s)
         break;
     case 'p':
         if (!strcmp(s, "owerctl")) return K_powerctl;
+        break;
     case 'r':
         if (!strcmp(s, "estart")) return K_restart;
         if (!strcmp(s, "estorecon")) return K_restorecon;
@@ -209,8 +210,7 @@ static int lookup_keyword(const char *s)
     return K_UNKNOWN;
 }
 
-static void parse_line_no_op(struct parse_state *state, int nargs, char **args)
-{
+static void parse_line_no_op(struct parse_state*, int, char**) {
 }
 
 static int push_chars(char **dst, int *len, const char *chars, int cnt)
@@ -378,7 +378,7 @@ static void parse_new_section(struct parse_state *state, int kw,
     state->parse_line = parse_line_no_op;
 }
 
-static void parse_config(const char *fn, char *s)
+static void parse_config(const char *fn, const std::string& data)
 {
     struct parse_state state;
     struct listnode import_list;
@@ -389,7 +389,7 @@ static void parse_config(const char *fn, char *s)
     nargs = 0;
     state.filename = fn;
     state.line = 0;
-    state.ptr = s;
+    state.ptr = strdup(data.c_str());  // TODO: fix this code!
     state.nexttoken = 0;
     state.parse_line = parse_line_no_op;
 
@@ -427,7 +427,6 @@ parser_done:
          struct import *import = node_to_item(node, struct import, list);
          int ret;
 
-         INFO("importing '%s'", import->filename);
          ret = init_parse_config_file(import->filename);
          if (ret)
              ERROR("could not import file '%s' from '%s'\n",
@@ -435,13 +434,14 @@ parser_done:
     }
 }
 
-int init_parse_config_file(const char *fn)
-{
-    char *data;
-    data = read_file(fn, 0);
-    if (!data) return -1;
+int init_parse_config_file(const char* path) {
+    INFO("Parsing %s...", path);
+    std::string data;
+    if (!read_file(path, &data)) {
+        return -1;
+    }
 
-    parse_config(fn, data);
+    parse_config(path, data);
     dump_parser_state();
     return 0;
 }
diff --git a/init/property_service.cpp b/init/property_service.cpp
index cc1ee34..05c03d6 100644
--- a/init/property_service.cpp
+++ b/init/property_service.cpp
@@ -30,6 +30,8 @@
 #include <cutils/sockets.h>
 #include <cutils/multiuser.h>
 
+#include <utils/file.h>
+
 #define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_
 #include <sys/_system_properties.h>
 
@@ -416,14 +418,9 @@ static void load_properties(char *data, const char *filter)
  */
 static void load_properties_from_file(const char *fn, const char *filter)
 {
-    char *data;
-    unsigned sz;
-
-    data = read_file(fn, &sz);
-
-    if(data != 0) {
-        load_properties(data, filter);
-        free(data);
+    std::string data;
+    if (read_file(fn, &data)) {
+        load_properties(&data[0], filter);
     }
 }
 
diff --git a/init/readme.txt b/init/readme.txt
index e5406ed..16a9186 100644
--- a/init/readme.txt
+++ b/init/readme.txt
@@ -191,10 +191,12 @@ enable <servicename>
      on property:ro.boot.myfancyhardware=1
         enable my_fancy_service_for_my_fancy_hardware
 
-
 insmod <path>
    Install the module at <path>
 
+loglevel <level>
+   Sets the kernel log level to level. Properties are expanded within <level>.
+
 mkdir <path> [mode] [owner] [group]
    Create a directory at <path>, optionally with the given mode, owner, and
    group. If not provided, the directory is created with permissions 755 and
@@ -229,7 +231,8 @@ setkey
    TBD
 
 setprop <name> <value>
-   Set system property <name> to <value>.
+   Set system property <name> to <value>. Properties are expanded
+   within <value>.
 
 setrlimit <resource> <cur> <max>
    Set the rlimit for a resource.
@@ -259,9 +262,10 @@ wait <path> [ <timeout> ]
   or the timeout has been reached. If timeout is not specified it
   currently defaults to five seconds.
 
-write <path> <string>
-   Open the file at <path> and write a string to it with write(2)
-   without appending.
+write <path> <content>
+   Open the file at <path> and write a string to it with write(2).
+   If the file does not exist, it will be created. If it does exist,
+   it will be truncated. Properties are expanded within <content>.
 
 
 Properties
@@ -338,8 +342,23 @@ Debugging notes
 ---------------
 By default, programs executed by init will drop stdout and stderr into
 /dev/null. To help with debugging, you can execute your program via the
-Andoird program logwrapper. This will redirect stdout/stderr into the
+Android program logwrapper. This will redirect stdout/stderr into the
 Android logging system (accessed via logcat).
 
 For example
 service akmd /system/bin/logwrapper /sbin/akmd
+
+For quicker turnaround when working on init itself, use:
+
+  mm
+  m ramdisk-nodeps
+  m bootimage-nodeps
+  adb reboot bootloader
+  fastboot boot $ANDROID_PRODUCT_OUT/boot.img
+
+Alternatively, use the emulator:
+
+  emulator -partition-size 1024 -verbose -show-kernel -no-window
+
+You might want to call klog_set_level(6) after the klog_init() call
+so you see the kernel logging in dmesg (or the emulator output).
diff --git a/init/ueventd_parser.cpp b/init/ueventd_parser.cpp
index 2ae251f..7a4841f 100644
--- a/init/ueventd_parser.cpp
+++ b/init/ueventd_parser.cpp
@@ -67,9 +67,7 @@ static int lookup_keyword(const char *s)
     return K_UNKNOWN;
 }
 
-static void parse_line_no_op(struct parse_state *state __attribute__((unused)),
-        int nargs __attribute__((unused)), char **args  __attribute__((unused)))
-{
+static void parse_line_no_op(struct parse_state*, int, char**) {
 }
 
 static int valid_name(const char *name)
@@ -97,9 +95,7 @@ struct ueventd_subsystem *ueventd_subsystem_find_by_name(const char *name)
     return 0;
 }
 
-static void *parse_subsystem(struct parse_state *state,
-        int nargs __attribute__((unused)), char **args)
-{
+static void *parse_subsystem(parse_state* state, int /*nargs*/, char** args) {
     if (!valid_name(args[1])) {
         parse_error(state, "invalid subsystem name '%s'\n", args[1]);
         return 0;
@@ -195,7 +191,7 @@ static void parse_line(struct parse_state *state, char **args, int nargs)
     }
 }
 
-static void parse_config(const char *fn, char *s)
+static void parse_config(const char *fn, const std::string& data)
 {
     struct parse_state state;
     char *args[UEVENTD_PARSER_MAXARGS];
@@ -203,7 +199,7 @@ static void parse_config(const char *fn, char *s)
     nargs = 0;
     state.filename = fn;
     state.line = 1;
-    state.ptr = s;
+    state.ptr = strdup(data.c_str());  // TODO: fix this code!
     state.nexttoken = 0;
     state.parse_line = parse_line_no_op;
     for (;;) {
@@ -230,17 +226,16 @@ static void parse_config(const char *fn, char *s)
 
 int ueventd_parse_config_file(const char *fn)
 {
-    char *data;
-    data = read_file(fn, 0);
-    if (!data) return -1;
+    std::string data;
+    if (!read_file(fn, &data)) {
+        return -1;
+    }
 
     parse_config(fn, data);
     dump_parser_state();
     return 0;
 }
 
-static void parse_line_device(struct parse_state *state __attribute__((unused)),
-        int nargs, char **args)
-{
+static void parse_line_device(parse_state*, int nargs, char** args) {
     set_device_permission(nargs, args);
 }
diff --git a/init/util.cpp b/init/util.cpp
index 84fe536..3dddb15 100644
--- a/init/util.cpp
+++ b/init/util.cpp
@@ -35,6 +35,8 @@
 /* for ANDROID_SOCKET_* */
 #include <cutils/sockets.h>
 
+#include <utils/file.h>
+
 #include <private/android_filesystem_config.h>
 
 #include "init.h"
@@ -146,48 +148,42 @@ out_close:
     return -1;
 }
 
-/* reads a file, making sure it is terminated with \n \0 */
-char *read_file(const char *fn, unsigned *_sz)
-{
-    char *data = NULL;
-    int sz;
-    int fd;
-    struct stat sb;
+bool read_file(const char* path, std::string* content) {
+    content->clear();
 
-    data = 0;
-    fd = open(fn, O_RDONLY|O_CLOEXEC);
-    if(fd < 0) return 0;
+    int fd = TEMP_FAILURE_RETRY(open(path, O_RDONLY|O_NOFOLLOW|O_CLOEXEC));
+    if (fd == -1) {
+        return false;
+    }
 
-    // for security reasons, disallow world-writable
-    // or group-writable files
-    if (fstat(fd, &sb) < 0) {
-        ERROR("fstat failed for '%s'\n", fn);
-        goto oops;
+    // For security reasons, disallow world-writable
+    // or group-writable files.
+    struct stat sb;
+    if (fstat(fd, &sb) == -1) {
+        ERROR("fstat failed for '%s': %s\n", path, strerror(errno));
+        return false;
     }
     if ((sb.st_mode & (S_IWGRP | S_IWOTH)) != 0) {
-        ERROR("skipping insecure file '%s'\n", fn);
-        goto oops;
+        ERROR("skipping insecure file '%s'\n", path);
+        return false;
     }
 
-    sz = lseek(fd, 0, SEEK_END);
-    if(sz < 0) goto oops;
-
-    if(lseek(fd, 0, SEEK_SET) != 0) goto oops;
-
-    data = (char*) malloc(sz + 2);
-    if(data == 0) goto oops;
-
-    if(read(fd, data, sz) != sz) goto oops;
-    close(fd);
-    data[sz] = '\n';
-    data[sz+1] = 0;
-    if(_sz) *_sz = sz;
-    return data;
+    bool okay = android::ReadFdToString(fd, content);
+    TEMP_FAILURE_RETRY(close(fd));
+    if (okay) {
+        content->append("\n", 1);
+    }
+    return okay;
+}
 
-oops:
-    close(fd);
-    free(data);
-    return 0;
+int write_file(const char* path, const char* content) {
+    int fd = TEMP_FAILURE_RETRY(open(path, O_WRONLY|O_CREAT|O_NOFOLLOW|O_CLOEXEC, 0600));
+    if (fd == -1) {
+        return -errno;
+    }
+    int result = android::WriteStringToFd(content, fd) ? 0 : -errno;
+    TEMP_FAILURE_RETRY(close(fd));
+    return result;
 }
 
 #define MAX_MTD_PARTITIONS 16
diff --git a/init/util.h b/init/util.h
index 1f9ecbe..77da3ac 100644
--- a/init/util.h
+++ b/init/util.h
@@ -20,6 +20,8 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#include <string>
+
 #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
 
 #define COLDBOOT_DONE "/dev/.coldboot_done"
@@ -27,7 +29,10 @@
 int mtd_name_to_number(const char *name);
 int create_socket(const char *name, int type, mode_t perm,
                   uid_t uid, gid_t gid, const char *socketcon);
-char *read_file(const char *fn, unsigned *_sz);
+
+bool read_file(const char* path, std::string* content);
+int write_file(const char* path, const char* content);
+
 time_t gettime(void);
 unsigned int decode_uid(const char *s);
 
diff --git a/init/util_test.cpp b/init/util_test.cpp
new file mode 100644
index 0000000..e9a1581
--- /dev/null
+++ b/init/util_test.cpp
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2015 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 "util.h"
+
+#include <errno.h>
+#include <gtest/gtest.h>
+
+TEST(util, read_file_ENOENT) {
+  std::string s("hello");
+  errno = 0;
+  EXPECT_FALSE(read_file("/proc/does-not-exist", &s));
+  EXPECT_EQ(ENOENT, errno);
+  EXPECT_EQ("", s); // s was cleared.
+}
+
+TEST(util, read_file_success) {
+  std::string s("hello");
+  EXPECT_TRUE(read_file("/proc/version", &s));
+  EXPECT_GT(s.length(), 6U);
+  EXPECT_EQ('\n', s[s.length() - 1]);
+  s[5] = 0;
+  EXPECT_STREQ("Linux", s.c_str());
+}