Simplify struct crash_info setup 16/225016/5
authorKarol Lewandowski <k.lewandowsk@samsung.com>
Fri, 14 Feb 2020 16:20:51 +0000 (17:20 +0100)
committerKarol Lewandowski <k.lewandowsk@samsung.com>
Tue, 18 Feb 2020 08:06:56 +0000 (09:06 +0100)
Change-Id: Ibbb13c4534b89d6e1ee4b951a438ae1748ae30d2

src/crash-manager/crash-manager.c
src/crash-manager/crash-manager.h

index 29337b7..ee717f2 100644 (file)
@@ -98,76 +98,69 @@ static int appinfo_get_appid_func(pkgmgrinfo_appinfo_h handle,
        return ret;
 }
 
-/* get application ID by pkgmgrinfo filter */
-static int get_appid(char *exepath, char *appid, int len)
+static bool get_appid(char *exepath, char **appid)
 {
        pkgmgrinfo_appinfo_filter_h handle = NULL;
-       int count, ret;
-       char *aid = NULL;
+       bool have_appid = false;
 
-       ret = pkgmgrinfo_appinfo_filter_create(&handle);
-       if (ret != PMINFO_R_OK) {
-               ret = -1;
-               goto out;
-       }
+       if (pkgmgrinfo_appinfo_filter_create(&handle) != PMINFO_R_OK)
+               return false;
 
-       ret = pkgmgrinfo_appinfo_filter_add_string(handle, PMINFO_APPINFO_PROP_APP_EXEC, exepath);
-       if (ret != PMINFO_R_OK) {
-               ret = -1;
+       if (pkgmgrinfo_appinfo_filter_add_string(handle, PMINFO_APPINFO_PROP_APP_EXEC, exepath) != PMINFO_R_OK)
                goto out_free;
-       }
 
-       ret = pkgmgrinfo_appinfo_filter_count(handle, &count);
-       if (ret != PMINFO_R_OK) {
-               ret = -1;
+       int count = 0;
+       if (pkgmgrinfo_appinfo_filter_count(handle, &count) || count < 1)
                goto out_free;
-       }
 
-       if (count < 1) {
-               ret = -1;
+       char *aid = NULL;
+       if (pkgmgrinfo_appinfo_filter_foreach_appinfo(handle, appinfo_get_appid_func, &aid) != PMINFO_R_OK)
                goto out_free;
-       }
 
-       ret = pkgmgrinfo_appinfo_filter_foreach_appinfo(handle, appinfo_get_appid_func, &aid);
-       if (ret != PMINFO_R_OK) {
-               ret = -1;
-               goto out_free;
-       }
-       if (aid) {
-               snprintf(appid, len, "%s", aid);
-               ret = 0;
-               free(aid);
-       }
+       have_appid = aid != NULL;
+       if (have_appid)
+               *appid = aid;
 
 out_free:
        pkgmgrinfo_appinfo_filter_destroy(handle);
-out:
-       return ret;
+       return have_appid;
 }
 
-/* get package ID by appid */
-static int get_pkgid(char *appid, char *pkgid, int len)
+static bool get_pkgid(char *appid, char **pkgid)
 {
        pkgmgrinfo_appinfo_h handle = NULL;
-       int ret;
-       char *pkid = NULL;
 
-       ret = pkgmgrinfo_appinfo_get_appinfo(appid, &handle);
-       if (ret != PMINFO_R_OK) {
-               ret = -1;
-               goto out;
-       }
-       ret = pkgmgrinfo_appinfo_get_pkgid(handle, &pkid);
-       if (ret != PMINFO_R_OK) {
-               ret = -1;
+       if (pkgmgrinfo_appinfo_get_appinfo(appid, &handle) != PMINFO_R_OK)
+               return false;
+
+       bool have_pkgid = false;
+       char *p = NULL;
+       if (pkgmgrinfo_appinfo_get_pkgid(handle, &p) != PMINFO_R_OK)
                goto out_free;
-       }
-       snprintf(pkgid, len, "%s", pkid);
+
+       have_pkgid = p != NULL;
+       if (have_pkgid)
+               *pkgid = p;
 
 out_free:
        pkgmgrinfo_appinfo_destroy_appinfo(handle);
-out:
-       return ret;
+       return have_pkgid;
+}
+
+static bool set_appinfo(char *exepath, char **appid, char **pkgid)
+{
+       char *a = NULL, *p = NULL;
+
+       if (get_appid(exepath, &a)) {
+               *appid = a;
+               *pkgid = get_pkgid(a, &p) ? p : strdup(a);
+               return true;
+       }
+
+       char *bn = basename(exepath);
+       *appid = strdup(bn);
+       *pkgid = strdup(bn);
+       return false;
 }
 
 static int prepare_paths(struct crash_info* cinfo)
@@ -308,127 +301,76 @@ close_fd:
        return -1;
 }
 
-bool set_crash_info(struct crash_info *cinfo)
-{
-       int ret;
-       char *temp_dir_ret = NULL;
-       char date[80];
-       struct tm loc_tm;
 
+static void set_crash_info_defaults(struct crash_info *cinfo)
+{
        if (cinfo->livedump) {
-               if (cinfo->kill)
-                       cinfo->sig_info = 9;
-               else
-                       cinfo->sig_info = 0;
-       }
-
-       if (cinfo->livedump && !file_exists(LIVEDUMPER_BIN_PATH)) {
-               fprintf(stderr, "Error: %s doesn't exist - can not perform livedump. Terminating.\n", LIVEDUMPER_BIN_PATH);
-               _E("Error: %s doesn't exist - can not perform livedump. Terminating.\n", LIVEDUMPER_BIN_PATH);
-               return false;
-       }
+               cinfo->sig_info = cinfo->kill ? 9 : 0;
+       } else if (cinfo->tid_info == -1)
+               cinfo->tid_info = find_crash_tid(cinfo->pid_info);
 
        if (cinfo->tid_info == -1) {
-               if (cinfo->livedump) {
-                       cinfo->tid_info = cinfo->pid_info;
-               } else {
-                       cinfo->tid_info = find_crash_tid(cinfo->pid_info);
-                       if (cinfo->tid_info < 0) {
-                               _W("TID not found. Assuming TID = PID (%d).", cinfo->pid_info);
-                               cinfo->tid_info = cinfo->pid_info;
-                       }
-               }
+               _W("TID not known. Assuming TID = PID (%d).", cinfo->pid_info);
+               cinfo->tid_info = cinfo->pid_info;
        }
+}
+
+/* Note: caller of this function is responsible for cleaning up the cinfo on failure */
+bool set_crash_info(struct crash_info *cinfo)
+{
+       set_crash_info_defaults(cinfo);
 
        if (!get_cmd_info(cinfo)) {
                _E("Failed to get command info");
                return false;
        }
 
-       if (cinfo->time_info == 0)
-               cinfo->time_info = time(NULL);
-
+       char date[80];
+       struct tm loc_tm;
+       cinfo->time_info = time(NULL);
        localtime_r(&cinfo->time_info, &loc_tm);
        strftime(date, sizeof(date), "%Y%m%d%H%M%S", &loc_tm);
 
-       if (asprintf(&cinfo->temp_dir, "%s/crash.XXXXXX", crash_temp_path) == -1) {
-               _E("Failed to asprintf for temp_dir");
-               cinfo->temp_dir = NULL;
-               return false;
-       }
+       if (asprintf(&cinfo->temp_dir, "%s/crash.XXXXXX", crash_temp_path) == -1)
+               goto out_oom;
 
-       temp_dir_ret = mkdtemp(cinfo->temp_dir);
-       if (!temp_dir_ret || access(temp_dir_ret, F_OK)) {
-               _E("Failed to mkdtemp for temp_dir");
+       if (mkdtemp(cinfo->temp_dir) == NULL || access(cinfo->temp_dir, F_OK)) {
+               _E("Failed to create temporary directory %s: %m", cinfo->temp_dir);
                return false;
        }
 
-       bool is_app = false;
-       if (get_appid(cinfo->cmd_line, cinfo->appid, sizeof(cinfo->appid)) < 0) {
-               snprintf(cinfo->appid, sizeof(cinfo->appid), "%s", basename(cinfo->cmd_line));
-               snprintf(cinfo->pkgid, sizeof(cinfo->pkgid), "%s", basename(cinfo->cmd_line));
-       } else {
-               is_app = true;
-               if (get_pkgid(cinfo->appid, cinfo->pkgid, sizeof(cinfo->pkgid)) < 0)
-                       snprintf(cinfo->pkgid, sizeof(cinfo->pkgid), "%s", cinfo->appid);
-       }
-
-       if (asprintf(&cinfo->name, "%s_%d_%s", is_app ? cinfo->pkgid : basename(cinfo->cmd_path),
-                       cinfo->pid_info, date) == -1) {
-               _E("Failed to snprintf for name");
-               cinfo->name = NULL;
-               goto rm_temp;
-       }
-
-       if (asprintf(&cinfo->pfx, "%s/%s", cinfo->temp_dir, cinfo->name) == -1) {
-               _E("Failed to asprintf for pfx");
-               cinfo->pfx = NULL;
-               goto rm_temp;
-       }
-       ret = mkdir(cinfo->pfx, 0775);
-       if (ret < 0) {
-               _E("Failed to mkdir for %s", cinfo->pfx);
-               goto rm_temp;
-       }
+       const char *suffix = config.report_type >= REP_TYPE_FULL ? (config.allow_zip ? ".zip" : "") : ".info";
+       bool is_app = set_appinfo(cinfo->cmd_line, &cinfo->appid, &cinfo->pkgid);
+       if (!cinfo->appid || !cinfo->pkgid)
+               goto out_oom;
 
-       if (asprintf(&cinfo->info_path, "%s/%s.info", cinfo->pfx, cinfo->name) == -1) {
-               _E("Failed to asprintf for info path");
-               cinfo->info_path = NULL;
-               goto rm_temp;
-       }
-
-       if (asprintf(&cinfo->core_path, "%s/%s.coredump", cinfo->pfx, cinfo->name) == -1) {
-               _E("Failed to asprintf for core path");
-               cinfo->core_path = NULL;
-               goto rm_temp;
-       }
+       if (-1 == asprintf(&cinfo->name,        "%s_%d_%s",       is_app ? cinfo->pkgid : basename(cinfo->cmd_path), cinfo->pid_info, date)
+        || -1 == asprintf(&cinfo->pfx,         "%s/%s",          cinfo->temp_dir, cinfo->name)
+        || -1 == asprintf(&cinfo->info_path,   "%s/%s.info",     cinfo->pfx, cinfo->name)
+        || -1 == asprintf(&cinfo->core_path,   "%s/%s.coredump", cinfo->pfx, cinfo->name)
+        || -1 == asprintf(&cinfo->log_path,    "%s/%s.log",      cinfo->pfx, cinfo->name)
+        || -1 == asprintf(&cinfo->zip_path,    "%s/report.zip",  cinfo->temp_dir)
+        || -1 == asprintf(&cinfo->result_path, "%s/%s%s",        crash_dump_path, cinfo->name, suffix))
+               goto out_oom;
 
-       if (asprintf(&cinfo->log_path, "%s/%s.log", cinfo->pfx, cinfo->name) == -1) {
-               _E("Failed to asprintf for log path");
-               cinfo->log_path = NULL;
-               goto rm_temp;
-       }
 
-       if (asprintf(&cinfo->zip_path, "%s/report.zip", cinfo->temp_dir) == -1) {
-               _E("Out of memory");
-               goto rm_temp;
+       if (mkdir(cinfo->pfx, 0775) < 0) {
+               _E("Failed to mkdir %s: %m", cinfo->pfx);
+               goto out_rm_temp;
        }
 
-       bool is_fullreport = config.report_type >= REP_TYPE_FULL;
-       const char *suffix = is_fullreport ? (config.allow_zip ? ".zip" : "") : ".info";
-
-       ret = asprintf(&cinfo->result_path, "%s/%s%s", crash_dump_path, cinfo->name, suffix);
-       if (ret == -1)
-               goto rm_temp;
-
        if (set_prstatus(cinfo) < 0)
-               goto rm_temp;
+               goto out_rm_temp;
 
        return true;
 
-rm_temp:
+out_rm_temp:
        remove_dir(cinfo->temp_dir, 1);
        return false;
+
+out_oom:
+       _E("Out of memory");
+       return false;
 }
 
 static void launch_crash_popup(struct crash_info *cinfo)
@@ -1032,6 +974,8 @@ static void free_crash_info(struct crash_info *cinfo)
        free(cinfo->core_path);
        free(cinfo->log_path);
        free(cinfo->zip_path);
+       free(cinfo->appid);
+       free(cinfo->pkgid);
 }
 
 void crash_info_init(struct crash_info *cinfo)
@@ -1052,6 +996,8 @@ void crash_info_init(struct crash_info *cinfo)
        cinfo->core_path = NULL;
        cinfo->log_path = NULL;
        cinfo->zip_path = NULL;
+       cinfo->appid = NULL;
+       cinfo->pkgid = NULL;
 }
 
 static bool run(struct crash_info *cinfo)
index 275fb5a..7e284e8 100644 (file)
@@ -23,9 +23,6 @@
 #include <unistd.h>
 #include <stdbool.h>
 
-#define APPID_MAX                   128
-#define PKGNAME_MAX                 128
-
 /* Paths and variables */
 struct crash_info {
        bool livedump;
@@ -48,8 +45,8 @@ struct crash_info {
        char *log_path;
        char *zip_path;
        char *output_path;
-       char appid[APPID_MAX];
-       char pkgid[PKGNAME_MAX];
+       char *appid;
+       char *pkgid;
        time_t time_info;
 };