logcat: Minor fixes

Remove -r default value implementation (since is not functional).
Check the status of some memory allocations.
Make sure that the pruning buffer is set to NULL if we get to the end of  the loop.
Make sure that the file status is successfully retrieved before using the size.
Add strtouia - string to unsigned argument.
Add logcat_panic with/without printing an error message and help.
Play nicer with the android namespace.

Change-Id: Id4c37cf64968128424e29bf1725f6c9a5924a0e2
Signed-off-by: Traian Schiau <traianx.schiau@intel.com>

1 files changed, 153 insertions, 133 deletions

diff --git a/logcat/logcat.cpp b/logcat/logcat.cpp
index be96fc4..5db539f 100644
--- a/logcat/logcat.cpp
+++ b/logcat/logcat.cpp
@@ -12,6 +12,7 @@
 #include <signal.h>
 #include <time.h>
 #include <unistd.h>
+#include <sys/cdefs.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <arpa/inet.h>
@@ -24,7 +25,6 @@
 #include <log/logprint.h>
 #include <log/event_tag_map.h>
 
-#define DEFAULT_LOG_ROTATE_SIZE_KBYTES 16
 #define DEFAULT_MAX_ROTATED_LOGS 4
 
 static AndroidLogFormat * g_logformat;
@@ -46,6 +46,8 @@ struct log_device_t {
         binary = b;
         next = NULL;
         printed = false;
+        logger = NULL;
+        logger_list = NULL;
     }
 };
 
@@ -54,13 +56,17 @@ namespace android {
 /* Global Variables */
 
 static const char * g_outputFileName = NULL;
-static int g_logRotateSizeKBytes = 0;                   // 0 means "no log rotation"
-static int g_maxRotatedLogs = DEFAULT_MAX_ROTATED_LOGS; // 0 means "unbounded"
+// 0 means "no log rotation"
+static size_t g_logRotateSizeKBytes = 0;
+// 0 means "unbounded"
+static size_t g_maxRotatedLogs = DEFAULT_MAX_ROTATED_LOGS;
 static int g_outFD = -1;
-static off_t g_outByteCount = 0;
+static size_t g_outByteCount = 0;
 static int g_printBinary = 0;
 static int g_devCount = 0;                              // >1 means multiple
 
+__noreturn static void logcat_panic(bool showHelp, const char *fmt, ...) __printflike(2,3);
+
 static int openLogFile (const char *pathname)
 {
     return open(pathname, O_WRONLY | O_APPEND | O_CREAT, S_IRUSR | S_IWUSR);
@@ -93,7 +99,12 @@ static void rotateLogs()
             asprintf(&file0, "%s.%.*d", g_outputFileName, maxRotationCountDigits, i - 1);
         }
 
-        err = rename (file0, file1);
+        if (!file0 || !file1) {
+            perror("while rotating log files");
+            break;
+        }
+
+        err = rename(file0, file1);
 
         if (err < 0 && errno != ENOENT) {
             perror("while rotating log files");
@@ -103,11 +114,10 @@ static void rotateLogs()
         free(file0);
     }
 
-    g_outFD = openLogFile (g_outputFileName);
+    g_outFD = openLogFile(g_outputFileName);
 
     if (g_outFD < 0) {
-        perror ("couldn't open output file");
-        exit(-1);
+        logcat_panic(false, "couldn't open output file");
     }
 
     g_outByteCount = 0;
@@ -153,8 +163,7 @@ static void processBuffer(log_device_t* dev, struct log_msg *buf)
         bytesWritten = android_log_printLogLine(g_logformat, g_outFD, &entry);
 
         if (bytesWritten < 0) {
-            perror("output error");
-            exit(-1);
+            logcat_panic(false, "output error");
         }
     }
 
@@ -179,8 +188,7 @@ static void maybePrintStart(log_device_t* dev, bool printDividers) {
                      dev->printed ? "switch to" : "beginning of",
                      dev->device);
             if (write(g_outFD, buf, strlen(buf)) < 0) {
-                perror("output error");
-                exit(-1);
+                logcat_panic(false, "output error");
             }
         }
         dev->printed = true;
@@ -199,11 +207,18 @@ static void setupOutput()
         g_outFD = openLogFile (g_outputFileName);
 
         if (g_outFD < 0) {
-            perror ("couldn't open output file");
-            exit(-1);
+            logcat_panic(false, "couldn't open output file");
         }
 
-        fstat(g_outFD, &statbuf);
+        if (fstat(g_outFD, &statbuf) == -1) {
+            close(g_outFD);
+            logcat_panic(false, "couldn't get output file stat\n");
+        }
+
+        if ((size_t) statbuf.st_size > SIZE_MAX || statbuf.st_size < 0) {
+            close(g_outFD);
+            logcat_panic(false, "invalid output file stat\n");
+        }
 
         g_outByteCount = statbuf.st_size;
     }
@@ -217,7 +232,7 @@ static void show_help(const char *cmd)
                     "  -s              Set default filter to silent.\n"
                     "                  Like specifying filterspec '*:S'\n"
                     "  -f <filename>   Log to file. Default to stdout\n"
-                    "  -r [<kbytes>]   Rotate log every kbytes. (16 if unspecified). Requires -f\n"
+                    "  -r <kbytes>     Rotate log every kbytes. Requires -f\n"
                     "  -n <count>      Sets max number of rotated logs to <count>, default 4\n"
                     "  -v <format>     Sets the log print format, where <format> is:\n\n"
                     "                  brief color long process raw tag thread threadtime time\n\n"
@@ -265,9 +280,6 @@ static void show_help(const char *cmd)
                    "or defaults to \"threadtime\"\n\n");
 }
 
-
-} /* namespace android */
-
 static int setLogFormat(const char * formatString)
 {
     static AndroidLogPrintFormat format;
@@ -308,8 +320,48 @@ static const char *multiplier_of_size(unsigned long value)
     return multipliers[i];
 }
 
+/*String to unsigned int, returns -1 if it fails*/
+static bool getSizeTArg(char *ptr, size_t *val, size_t min = 0,
+                        size_t max = SIZE_MAX)
+{
+    char *endp;
+    errno = 0;
+    size_t ret = (size_t) strtoll(ptr, &endp, 0);
+
+    if (endp[0] != '\0' || errno != 0 ) {
+        return false;
+    }
+
+    if (ret >  max || ret <  min) {
+        return false;
+    }
+
+    *val = ret;
+    return true;
+}
+
+static void logcat_panic(bool showHelp, const char *fmt, ...)
+{
+    if (fmt) {
+        va_list  args;
+        va_start(args, fmt);
+        vfprintf(stderr, fmt,  args);
+        va_end(args);
+    }
+
+    if (showHelp) {
+       show_help(getprogname());
+    }
+
+    exit(EXIT_FAILURE);
+}
+
+} /* namespace android */
+
+
 int main(int argc, char **argv)
 {
+    using namespace android;
     int err;
     int hasSetLogFormat = 0;
     int clearLog = 0;
@@ -324,7 +376,7 @@ int main(int argc, char **argv)
     log_device_t* dev;
     bool printDividers = false;
     struct logger_list *logger_list;
-    unsigned int tail_lines = 0;
+    size_t tail_lines = 0;
     log_time tail_time(log_time::EPOCH);
 
     signal(SIGPIPE, exit);
@@ -332,14 +384,14 @@ int main(int argc, char **argv)
     g_logformat = android_log_format_new();
 
     if (argc == 2 && 0 == strcmp(argv[1], "--help")) {
-        android::show_help(argv[0]);
-        exit(0);
+        show_help(argv[0]);
+        return EXIT_SUCCESS;
     }
 
     for (;;) {
         int ret;
 
-        ret = getopt(argc, argv, "cdDLt:T:gG:sQf:r:n:v:b:BSpP:");
+        ret = getopt(argc, argv, ":cdDLt:T:gG:sQf:r:n:v:b:BSpP:");
 
         if (ret < 0) {
             break;
@@ -372,10 +424,9 @@ int main(int argc, char **argv)
                     char *cp = tail_time.strptime(optarg,
                                                   log_time::default_format);
                     if (!cp) {
-                        fprintf(stderr,
-                                "ERROR: -%c \"%s\" not in \"%s\" time format\n",
-                                ret, optarg, log_time::default_format);
-                        exit(1);
+                        logcat_panic(false,
+                                    "-%c \"%s\" not in \"%s\" time format\n",
+                                    ret, optarg, log_time::default_format);
                     }
                     if (*cp) {
                         char c = *cp;
@@ -386,8 +437,7 @@ int main(int argc, char **argv)
                         *cp = c;
                     }
                 } else {
-                    tail_lines = atoi(optarg);
-                    if (!tail_lines) {
+                    if (!getSizeTArg(optarg, &tail_lines, 1)) {
                         fprintf(stderr,
                                 "WARNING: -%c %s invalid, setting to 1\n",
                                 ret, optarg);
@@ -405,13 +455,11 @@ int main(int argc, char **argv)
             break;
 
             case 'G': {
-                // would use atol if not for the multiplier
-                char *cp = optarg;
-                setLogSize = 0;
-                while (('0' <= *cp) && (*cp <= '9')) {
-                    setLogSize *= 10;
-                    setLogSize += *cp - '0';
-                    ++cp;
+                char *cp;
+                if (strtoll(optarg, &cp, 0) > 0) {
+                    setLogSize = strtoll(optarg, &cp, 0);
+                } else {
+                    setLogSize = 0;
                 }
 
                 switch(*cp) {
@@ -436,7 +484,7 @@ int main(int argc, char **argv)
 
                 if (!setLogSize) {
                     fprintf(stderr, "ERROR: -G <num><multiplier>\n");
-                    exit(1);
+                    return EXIT_FAILURE;
                 }
             }
             break;
@@ -458,7 +506,7 @@ int main(int argc, char **argv)
                     }
 
                     devices = dev = NULL;
-                    android::g_devCount = 0;
+                    g_devCount = 0;
                     for(int i = LOG_ID_MIN; i < LOG_ID_MAX; ++i) {
                         const char *name = android_log_id_to_name((log_id_t)i);
                         log_id_t log_id = android_name_to_log_id(name);
@@ -476,7 +524,7 @@ int main(int argc, char **argv)
                         } else {
                             devices = dev = d;
                         }
-                        android::g_devCount++;
+                        g_devCount++;
                     }
                     break;
                 }
@@ -492,51 +540,36 @@ int main(int argc, char **argv)
                 } else {
                     devices = new log_device_t(optarg, binary);
                 }
-                android::g_devCount++;
+                g_devCount++;
             }
             break;
 
             case 'B':
-                android::g_printBinary = 1;
+                g_printBinary = 1;
             break;
 
             case 'f':
                 // redirect output to a file
-
-                android::g_outputFileName = optarg;
+                g_outputFileName = optarg;
 
             break;
 
             case 'r':
-                if (optarg == NULL) {
-                    android::g_logRotateSizeKBytes
-                                = DEFAULT_LOG_ROTATE_SIZE_KBYTES;
-                } else {
-                    if (!isdigit(optarg[0])) {
-                        fprintf(stderr,"Invalid parameter to -r\n");
-                        android::show_help(argv[0]);
-                        exit(-1);
-                    }
-                    android::g_logRotateSizeKBytes = atoi(optarg);
+                if (!getSizeTArg(optarg, &g_logRotateSizeKBytes, 1)) {
+                    logcat_panic(true, "Invalid parameter %s to -r\n", optarg);
                 }
             break;
 
             case 'n':
-                if (!isdigit(optarg[0])) {
-                    fprintf(stderr,"Invalid parameter to -r\n");
-                    android::show_help(argv[0]);
-                    exit(-1);
+                if (!getSizeTArg(optarg, &g_maxRotatedLogs, 1)) {
+                    logcat_panic(true, "Invalid parameter %s to -n\n", optarg);
                 }
-
-                android::g_maxRotatedLogs = atoi(optarg);
             break;
 
             case 'v':
                 err = setLogFormat (optarg);
                 if (err < 0) {
-                    fprintf(stderr,"Invalid parameter to -v\n");
-                    android::show_help(argv[0]);
-                    exit(-1);
+                    logcat_panic(true, "Invalid parameter %s to -v\n", optarg);
                 }
 
                 if (strcmp("color", optarg)) { // exception for modifiers
@@ -582,8 +615,9 @@ int main(int argc, char **argv)
                         force_exit   = 0;
                     }
                     /* if nothing found or invalid filters, exit quietly */
-                    if (force_exit)
-                        exit(0);
+                    if (force_exit) {
+                        return EXIT_SUCCESS;
+                    }
 
                     /* redirect our output to the emulator console */
                     if (console) {
@@ -615,36 +649,34 @@ int main(int argc, char **argv)
                 printStatistics = 1;
                 break;
 
+            case ':':
+                logcat_panic(true, "Option -%c needs an argument\n", optopt);
+                break;
+
             default:
-                fprintf(stderr,"Unrecognized Option\n");
-                android::show_help(argv[0]);
-                exit(-1);
-            break;
+                logcat_panic(true, "Unrecognized Option %c\n", optopt);
+                break;
         }
     }
 
     if (!devices) {
         dev = devices = new log_device_t("main", false);
-        android::g_devCount = 1;
+        g_devCount = 1;
         if (android_name_to_log_id("system") == LOG_ID_SYSTEM) {
             dev = dev->next = new log_device_t("system", false);
-            android::g_devCount++;
+            g_devCount++;
         }
         if (android_name_to_log_id("crash") == LOG_ID_CRASH) {
             dev = dev->next = new log_device_t("crash", false);
-            android::g_devCount++;
+            g_devCount++;
         }
     }
 
-    if (android::g_logRotateSizeKBytes != 0
-        && android::g_outputFileName == NULL
-    ) {
-        fprintf(stderr,"-r requires -f as well\n");
-        android::show_help(argv[0]);
-        exit(-1);
+    if (g_logRotateSizeKBytes != 0 && g_outputFileName == NULL) {
+        logcat_panic(true, "-r requires -f as well\n");
     }
 
-    android::setupOutput();
+    setupOutput();
 
     if (hasSetLogFormat == 0) {
         const char* logFormat = getenv("ANDROID_PRINTF_LOG");
@@ -663,8 +695,7 @@ int main(int argc, char **argv)
     if (forceFilters) {
         err = android_log_addFilterString(g_logformat, forceFilters);
         if (err < 0) {
-            fprintf (stderr, "Invalid filter expression in -logcat option\n");
-            exit(0);
+            logcat_panic(false, "Invalid filter expression in logcat args\n");
         }
     } else if (argc == optind) {
         // Add from environment variable
@@ -674,10 +705,8 @@ int main(int argc, char **argv)
             err = android_log_addFilterString(g_logformat, env_tags_orig);
 
             if (err < 0) {
-                fprintf(stderr, "Invalid filter expression in"
-                                    " ANDROID_LOG_TAGS\n");
-                android::show_help(argv[0]);
-                exit(-1);
+                logcat_panic(true,
+                            "Invalid filter expression in ANDROID_LOG_TAGS\n");
             }
         }
     } else {
@@ -686,9 +715,7 @@ int main(int argc, char **argv)
             err = android_log_addFilterString(g_logformat, argv[i]);
 
             if (err < 0) {
-                fprintf (stderr, "Invalid filter expression '%s'\n", argv[i]);
-                android::show_help(argv[0]);
-                exit(-1);
+                logcat_panic(true, "Invalid filter expression '%s'\n", argv[i]);
             }
         }
     }
@@ -704,22 +731,20 @@ int main(int argc, char **argv)
         dev->logger = android_logger_open(logger_list,
                                           android_name_to_log_id(dev->device));
         if (!dev->logger) {
-            fprintf(stderr, "Unable to open log device '%s'\n", dev->device);
-            exit(EXIT_FAILURE);
+            logcat_panic(false, "Unable to open log device '%s'\n",
+                         dev->device);
         }
 
         if (clearLog) {
             int ret;
             ret = android_logger_clear(dev->logger);
             if (ret) {
-                perror("failed to clear the log");
-                exit(EXIT_FAILURE);
+                logcat_panic(false, "failed to clear the log");
             }
         }
 
         if (setLogSize && android_logger_set_log_size(dev->logger, setLogSize)) {
-            perror("failed to set the log size");
-            exit(EXIT_FAILURE);
+            logcat_panic(false, "failed to set the log size");
         }
 
         if (getLogSize) {
@@ -727,14 +752,12 @@ int main(int argc, char **argv)
 
             size = android_logger_get_log_size(dev->logger);
             if (size < 0) {
-                perror("failed to get the log size");
-                exit(EXIT_FAILURE);
+                logcat_panic(false, "failed to get the log size");
             }
 
             readable = android_logger_get_log_readable_size(dev->logger);
             if (readable < 0) {
-                perror("failed to get the readable log size");
-                exit(EXIT_FAILURE);
+                logcat_panic(false, "failed to get the readable log size");
             }
 
             printf("%s: ring buffer is %ld%sb (%ld%sb consumed), "
@@ -748,16 +771,18 @@ int main(int argc, char **argv)
     }
 
     if (setPruneList) {
-        size_t len = strlen(setPruneList) + 32; // margin to allow rc
-        char *buf = (char *) malloc(len);
-
-        strcpy(buf, setPruneList);
-        int ret = android_logger_set_prune_list(logger_list, buf, len);
-        free(buf);
-
-        if (ret) {
-            perror("failed to set the prune list");
-            exit(EXIT_FAILURE);
+        size_t len = strlen(setPruneList);
+        /*extra 32 bytes are needed by  android_logger_set_prune_list */
+        size_t bLen = len + 32;
+        char *buf = NULL;
+        if (asprintf(&buf, "%-*s", (int)(bLen - 1), setPruneList) > 0) {
+            buf[len] = '\0';
+            if (android_logger_set_prune_list(logger_list, buf, bLen)) {
+                logcat_panic(false, "failed to set the prune list");
+            }
+            free(buf);
+        } else {
+            logcat_panic(false, "failed to set the prune list (alloc)");
         }
     }
 
@@ -767,29 +792,28 @@ int main(int argc, char **argv)
 
         for(int retry = 32;
                 (retry >= 0) && ((buf = new char [len]));
-                delete [] buf, --retry) {
+                delete [] buf, buf = NULL, --retry) {
             if (getPruneList) {
                 android_logger_get_prune_list(logger_list, buf, len);
             } else {
                 android_logger_get_statistics(logger_list, buf, len);
             }
             buf[len-1] = '\0';
-            size_t ret = atol(buf) + 1;
-            if (ret < 4) {
+            if (atol(buf) < 3) {
                 delete [] buf;
                 buf = NULL;
                 break;
             }
-            bool check = ret <= len;
-            len = ret;
-            if (check) {
+            size_t ret = atol(buf) + 1;
+            if (ret <= len) {
+                len = ret;
                 break;
             }
+            len = ret;
         }
 
         if (!buf) {
-            perror("failed to read data");
-            exit(EXIT_FAILURE);
+            logcat_panic(false, "failed to read data");
         }
 
         // remove trailing FF
@@ -813,18 +837,18 @@ int main(int argc, char **argv)
 
         printf("%s", cp);
         delete [] buf;
-        exit(0);
+        return EXIT_SUCCESS;
     }
 
 
     if (getLogSize) {
-        exit(0);
+        return EXIT_SUCCESS;
     }
     if (setLogSize || setPruneList) {
-        exit(0);
+        return EXIT_SUCCESS;
     }
     if (clearLog) {
-        exit(0);
+        return EXIT_SUCCESS;
     }
 
     //LOG_EVENT_INT(10, 12345);
@@ -839,8 +863,7 @@ int main(int argc, char **argv)
         int ret = android_logger_list_read(logger_list, &log_msg);
 
         if (ret == 0) {
-            fprintf(stderr, "read: unexpected EOF!\n");
-            exit(EXIT_FAILURE);
+            logcat_panic(false, "read: unexpected EOF!\n");
         }
 
         if (ret < 0) {
@@ -849,15 +872,12 @@ int main(int argc, char **argv)
             }
 
             if (ret == -EIO) {
-                fprintf(stderr, "read: unexpected EOF!\n");
-                exit(EXIT_FAILURE);
+                logcat_panic(false, "read: unexpected EOF!\n");
             }
             if (ret == -EINVAL) {
-                fprintf(stderr, "read: unexpected length.\n");
-                exit(EXIT_FAILURE);
+                logcat_panic(false, "read: unexpected length.\n");
             }
-            perror("logcat read failure");
-            exit(EXIT_FAILURE);
+            logcat_panic(false, "logcat read failure");
         }
 
         for(d = devices; d; d = d->next) {
@@ -866,23 +886,23 @@ int main(int argc, char **argv)
             }
         }
         if (!d) {
-            android::g_devCount = 2; // set to Multiple
+            g_devCount = 2; // set to Multiple
             d = &unexpected;
             d->binary = log_msg.id() == LOG_ID_EVENTS;
         }
 
         if (dev != d) {
             dev = d;
-            android::maybePrintStart(dev, printDividers);
+            maybePrintStart(dev, printDividers);
         }
-        if (android::g_printBinary) {
-            android::printBinary(&log_msg);
+        if (g_printBinary) {
+            printBinary(&log_msg);
         } else {
-            android::processBuffer(dev, &log_msg);
+            processBuffer(dev, &log_msg);
         }
     }
 
     android_logger_list_free(logger_list);
 
-    return 0;
+    return EXIT_SUCCESS;
 }