logcat: -f <non-existent-directory>/<filename> segfaults

(cherry pick from commit c18c2130d036b115b222860366a97e26a2a1c038)

- Check if the result of opendir is NULL in lastLogTime
- Cleanup: alphabetically sorted long options, reserved
  an alias for --regex
- Add a unit test, non existent directory should return
  gracefully with an exit(1) and not SIGSEGV.

NB: This failure was with eng/debug feature logpersist
    turned on, /data/misc/logd/ directory was missing,
    deleted, or temporarily inaccessible.

Bug: 27954627
Change-Id: I60246a53b02fdd7e5490fe458b02ad7b14843584

2 files changed, 25 insertions, 8 deletions

diff --git a/logcat/logcat.cpp b/logcat/logcat.cpp
index 15fc03541..37f4f4f27 100644
--- a/logcat/logcat.cpp
+++ b/logcat/logcat.cpp
@@ -451,10 +451,6 @@ static log_time lastLogTime(char *outputFileName) {
         return retval;
     }
 
-    clockid_t clock_type = android_log_clockid();
-    log_time now(clock_type);
-    bool monotonic = clock_type == CLOCK_MONOTONIC;
-
     std::string directory;
     char *file = strrchr(outputFileName, '/');
     if (!file) {
@@ -466,10 +462,21 @@ static log_time lastLogTime(char *outputFileName) {
         *file = '/';
         ++file;
     }
+
+    std::unique_ptr<DIR, int(*)(DIR*)>
+            dir(opendir(directory.c_str()), closedir);
+    if (!dir.get()) {
+        return retval;
+    }
+
+    clockid_t clock_type = android_log_clockid();
+    log_time now(clock_type);
+    bool monotonic = clock_type == CLOCK_MONOTONIC;
+
     size_t len = strlen(file);
     log_time modulo(0, NS_PER_SEC);
-    std::unique_ptr<DIR, int(*)(DIR*)>dir(opendir(directory.c_str()), closedir);
     struct dirent *dp;
+
     while ((dp = readdir(dir.get())) != NULL) {
         if ((dp->d_type != DT_REG)
                 // If we are using realtime, check all files that match the
@@ -578,11 +585,13 @@ int main(int argc, char **argv)
           { "dividers",      no_argument,       NULL,   'D' },
           { "file",          required_argument, NULL,   'f' },
           { "format",        required_argument, NULL,   'v' },
+          // hidden and undocumented reserved alias for --regex
+          { "grep",          required_argument, NULL,   'e' },
           // hidden and undocumented reserved alias for --max-count
           { "head",          required_argument, NULL,   'm' },
           { "last",          no_argument,       NULL,   'L' },
-          { pid_str,         required_argument, NULL,   0 },
           { "max-count",     required_argument, NULL,   'm' },
+          { pid_str,         required_argument, NULL,   0 },
           { print_str,       no_argument,       NULL,   0 },
           { "prune",         optional_argument, NULL,   'p' },
           { "regex",         required_argument, NULL,   'e' },
diff --git a/logcat/tests/logcat_test.cpp b/logcat/tests/logcat_test.cpp
index 28772095d..921a4618c 100644
--- a/logcat/tests/logcat_test.cpp
+++ b/logcat/tests/logcat_test.cpp
@@ -21,6 +21,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/types.h>
+#include <sys/wait.h>
 
 #include <gtest/gtest.h>
 #include <log/log.h>
@@ -781,8 +782,15 @@ TEST(logcat, logrotate_continue) {
     EXPECT_FALSE(system(command));
 }
 
-static void caught_blocking_clear(int /*signum*/)
-{
+TEST(logcat, logrotate_nodir) {
+    // expect logcat to error out on writing content and exit(1) for nodir
+    EXPECT_EQ(W_EXITCODE(1, 0),
+              system("logcat -b all -d"
+                     " -f /das/nein/gerfingerpoken/logcat/log.txt"
+                     " -n 256 -r 1024"));
+}
+
+static void caught_blocking_clear(int /*signum*/) {
     unsigned long long v = 0xDEADBEEFA55C0000ULL;
 
     v += getpid() & 0xFFFF;