Use parsed command line arguments instead of raw string values

Change-Id: I28c9479c2d6cb5d754e7ff3ee37f8cab34235456

diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c
index a778b97..3603907 100644 (file)
--- a/src/crash-manager/crash-manager.c
+++ b/src/crash-manager/crash-manager.c
@@ -113,12 +113,14 @@ static int report_type;
 
 /* Paths and variables */
 static struct crash_info {
-       char *pid_info;
-       char *tid_info;
-       char *sig_info;
+       pid_t pid_info;
+       pid_t tid_info;
+       int uid_info;
+       int gid_info;
+       int sig_info;
        char cmd_line[PATH_MAX];
        char cmd_path[PATH_MAX];
-       char time_info[80];
+       time_t time_info;
        char temp_dir[PATH_MAX];
        char name[FILENAME_MAX];
        char result_path[PATH_MAX];
@@ -437,7 +439,7 @@ static int set_prstatus()
        int ret;
        char prstatus_name[NAME_MAX+1];
 
-       ret = snprintf(prstatus_name, NAME_MAX, "/%s.prstatus", crash_info.pid_info);
+       ret = snprintf(prstatus_name, NAME_MAX, "/%d.prstatus", crash_info.pid_info);
        if (ret < 0) {
                _E("Failed to snprintf for prstatus path");
                goto close_fd;
@@ -485,29 +487,18 @@ static int set_crash_info(int argc, char *argv[])
 {
        int ret;
        char *temp_dir_ret = NULL;
-       time_t time_val;
+       char date[80];
        struct tm loc_tm;
 
-       crash_info.pid_info = argv[1];
-       crash_info.sig_info = argv[4];
-       if (argc > 6) {
-               crash_info.tid_info = strdup(argv[6]);
-               if (crash_info.tid_info == NULL) {
-                       _E("strdup error: %m");
-                       return -1;
-               }
-       } else {
-               crash_info.tid_info = NULL;
-               int pid = atoi(crash_info.pid_info);
-               int tid = find_crash_tid(pid);
-               if (tid < 0) {
+       crash_info.pid_info = strtol(argv[1], NULL, 10);
+       crash_info.sig_info = atoi(argv[4]);
+       if (argc > 6)
+               crash_info.tid_info = strtol(argv[6], NULL, 10);
+       else {
+               crash_info.tid_info = find_crash_tid(crash_info.pid_info);
+               if (crash_info.tid_info < 0) {
                        _I("TID not found");
-                       tid = pid;
-               }
-
-               if (asprintf(&crash_info.tid_info, "%d", tid) == -1) {
-                       _E("asprintf error: %m");
-                       return -1;
+                       crash_info.tid_info = crash_info.pid_info;
                }
        }
 
@@ -517,10 +508,9 @@ static int set_crash_info(int argc, char *argv[])
                return -1;
        }
 
-       time_val = atoll(argv[5]);
-       localtime_r(&time_val, &loc_tm);
-       strftime(crash_info.time_info, sizeof(crash_info.time_info),
-                       "%Y%m%d%H%M%S", &loc_tm);
+       crash_info.time_info = strtol(argv[5], NULL, 10);
+       localtime_r(&crash_info.time_info, &loc_tm);
+       strftime(date, sizeof(date), "%Y%m%d%H%M%S", &loc_tm);
 
        ret = snprintf(crash_info.temp_dir, sizeof(crash_info.temp_dir),
                        "%s/crash.XXXXXX", crash_temp_path);
@@ -534,10 +524,10 @@ static int set_crash_info(int argc, char *argv[])
                return -1;
        }
 
-       ret = snprintf(crash_info.name, sizeof(crash_info.name), "%s_%s_%s",
+       ret = snprintf(crash_info.name, sizeof(crash_info.name), "%s_%d_%s",
                        basename(crash_info.cmd_line),
                        crash_info.pid_info,
-                       crash_info.time_info);
+                       date);
        if (ret < 0) {
                _E("Failed to snprintf for name");
                goto rm_temp;
@@ -592,7 +582,7 @@ static int set_crash_info(int argc, char *argv[])
 #ifdef SYS_ASSERT
        ret = snprintf(crash_info.sysassert_cs_path,
                        sizeof(crash_info.sysassert_cs_path),
-                      "/tmp/crash_stack/%s_%s.info",
+                      "/tmp/crash_stack/%s_%d.info",
                       basename(crash_info.cmd_line), crash_info.pid_info);
        if (ret < 0) {
                _E("Failed to snprintf for sys-assert callstack path");
@@ -719,7 +709,7 @@ static void copy_maps()
 {
        char maps_path[PATH_MAX];
        char temp_maps_path[PATH_MAX];
-       snprintf(maps_path, sizeof(maps_path), "/proc/%s/maps",
+       snprintf(maps_path, sizeof(maps_path), "/proc/%d/maps",
                         crash_info.pid_info);
 
        snprintf(temp_maps_path, sizeof(temp_maps_path), "%s/%s",
@@ -751,27 +741,36 @@ static void save_so_info()
        get_and_save_so_info(maps_path, so_info_path);
 }
 
-static int execute_minicoredump(int argc, char *argv[])
+static int execute_minicoredump()
 {
+#define SNPRINTF_OR_EXIT(name, format) if (snprintf(name##_str, sizeof(name##_str), format, crash_info.name##_info) < 0) goto out;
+
        char *coredump_name = NULL;
        char *prstatus_fd_str = NULL;
-       int ret = 0;
+       int ret = -1;
 
        if (asprintf(&coredump_name, "%s.coredump", crash_info.name) == -1
            || asprintf(&prstatus_fd_str, "%d", crash_info.prstatus_fd) == -1) {
                _E("Unable to allocate memory");
-               ret = -1;
                goto out;
        }
 
+       char pid_str[11], uid_str[11], gid_str[11], sig_str[11], time_str[11];
+
+       SNPRINTF_OR_EXIT(pid, "%d")
+       SNPRINTF_OR_EXIT(uid, "%d")
+       SNPRINTF_OR_EXIT(gid, "%d")
+       SNPRINTF_OR_EXIT(sig, "%d")
+       SNPRINTF_OR_EXIT(time, "%ld")
+
        /* Execute minicoredumper */
        char *args[] = {
                        MINICOREDUMPER_BIN_PATH,   // minicoredumper filename path
-                       crash_info.pid_info,       // %p - pid
-                       argv[2],                   // %u - UID
-                       argv[3],                   // %g - GID
-                       crash_info.sig_info,       // %s - number of signal
-                       argv[5],                   // %t - time of dump
+                       pid_str,                   // %p - pid
+                       uid_str,                   // %u - UID
+                       gid_str,                   // %g - GID
+                       sig_str,                   // %s - number of signal
+                       time_str,                  // %t - time of dump
                        "localhost",               // %h - hostname
                        "core",                    // %e - exe name (need for result filename)
                        MINICOREDUMPER_CONFIG_PATH, // config file
@@ -791,6 +790,7 @@ static int execute_minicoredump(int argc, char *argv[])
                args[12], args[13], args[14]);
 
        run_command_timeout(args[0], args, NULL, MINICOREDUMPER_TIMEOUT);
+       ret = 0;
 
        /* Minicoredumper must be executed to dump at least PRSTATUS for
           other tools, coredump, however, might have been disabled. */
@@ -818,16 +818,17 @@ out:
        free(prstatus_fd_str);
 
        return ret;
+#undef SNPRINTF_OR_EXIT
 }
 
-static void execute_crash_stack(int argc, char *argv[])
+static void execute_crash_stack()
 {
        int ret;
        char command[PATH_MAX];
 
        /* Execute crash-stack */
        ret = snprintf(command, sizeof(command),
-                       "%s --pid %s --tid %s --sig %s --prstatus_fd %d > %s",
+                       "%s --pid %d --tid %d --sig %d --prstatus_fd %d > %s",
                        CRASH_STACK_PATH,
                        crash_info.pid_info,
                        crash_info.tid_info,
@@ -844,11 +845,11 @@ static void execute_crash_stack(int argc, char *argv[])
        system_command(command);
 }
 
-static int execute_crash_modules(int argc, char *argv[])
+static int execute_crash_modules()
 {
        _D("Execute crash module: ");
 
-       if (execute_minicoredump(argc, argv) < 0) {
+       if (execute_minicoredump() < 0) {
                _E("Failed to run minicoredumper - can not continue");
                return -1;
        }
@@ -859,7 +860,7 @@ static int execute_crash_modules(int argc, char *argv[])
        if (crash_info.have_sysassert_report)
                return -1;
 #endif
-       execute_crash_stack(argc, argv);
+       execute_crash_stack();
 
        return 0;
 }
@@ -1242,7 +1243,7 @@ int main(int argc, char *argv[])
                copy_maps();
        }
        /* Exec crash modules */
-       if (execute_crash_modules(argc, argv) < 0) {
+       if (execute_crash_modules() < 0) {
                res = EXIT_FAILURE;
                goto exit;
        }
@@ -1269,8 +1270,8 @@ int main(int argc, char *argv[])
 
        struct NotifyParams notify_params = {
                .prstatus_fd = crash_info.prstatus_fd,
-               .pid = strtol(crash_info.pid_info, NULL, 10),
-               .tid = strtol(crash_info.tid_info, NULL, 10),
+               .pid = crash_info.pid_info,
+               .tid = crash_info.tid_info,
                .cmd_name = basename(crash_info.cmd_line),
                .cmd_path = crash_info.cmd_path,
                .report_path = crash_info.result_path,
@@ -1282,7 +1283,6 @@ int main(int argc, char *argv[])
 
 exit:
        close(crash_info.prstatus_fd);
-       free(crash_info.tid_info);
        free(crash_temp_path);
        free(crash_root_path);
        free(crash_crash_path);

diff --git a/src/crash-stack/crash-stack.c b/src/crash-stack/crash-stack.c
index 2f65b7c..35516eb 100644 (file)
--- a/src/crash-stack/crash-stack.c
+++ b/src/crash-stack/crash-stack.c
@@ -259,11 +259,8 @@ void callstack_destructor(Callstack *callstack)
 static void __crash_stack_print_exe(FILE* outputfile, pid_t pid)
 {
        char file_path[PATH_MAX];
-       char pid_str[11];
 
-       snprintf(pid_str, sizeof(pid_str), "%d", pid);
-
-       if (get_exe_path(pid_str, file_path, sizeof(file_path)) == 0)
+       if (get_exe_path(pid, file_path, sizeof(file_path)) == 0)
                snprintf(file_path, sizeof(file_path), "<unknown>");
 
        fprintf(outputfile, "Executable File Path: %s\n", file_path);

diff --git a/src/shared/util.c b/src/shared/util.c
index 2eb4399..97f3499 100644 (file)
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -819,12 +819,12 @@ int find_crash_tid(int pid)
        return -1;
 }
 
-int get_cmd_line(char* pid, char *cmd, size_t len)
+int get_cmd_line(pid_t pid, char *cmd, size_t len)
 {
        char cmdline_path[PATH_MAX];
 
        snprintf(cmdline_path, sizeof(cmdline_path),
-                       "/proc/%s/cmdline", pid);
+                       "/proc/%d/cmdline", pid);
 
        int fd = open(cmdline_path, O_RDONLY);
        if (fd < 0) {
@@ -845,12 +845,12 @@ int get_cmd_line(char* pid, char *cmd, size_t len)
        return 1;
 }
 
-int get_exe_path(char* pid, char *path, size_t len)
+int get_exe_path(pid_t pid, char *path, size_t len)
 {
        char exe_link[PATH_MAX];
 
        snprintf(exe_link, sizeof(exe_link),
-               "/proc/%s/exe", pid);
+               "/proc/%d/exe", pid);
 
        ssize_t ret = readlink(exe_link, path, len - 1);
 

diff --git a/src/shared/util.h b/src/shared/util.h
index 0cb35f2..5d40d91 100644 (file)
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -63,9 +63,9 @@ int log_kmsg(char *fmt, ...);
 
 int find_crash_tid(int pid);
 
-int get_exe_path(char* pid, char *path, size_t len);
+int get_exe_path(pid_t pid, char *path, size_t len);
 
-int get_cmd_line(char* pid, char *cmd, size_t len);
+int get_cmd_line(pid_t pid, char *cmd, size_t len);
 /**
  * @}
  */