create_dump_dir_from_problem_data() adopts almost all new tricks from
save_dump_dir_from_problem_data(), but still returns struct dump_dir * -
the caller might want to use dd w/o the need to lock it again.

The libreport patch goes first. abrt one will follow.
-- 
vda


diff -x '*.po' -d -urpN libreport.8/src/include/problem_data.h 
libreport.9/src/include/problem_data.h
--- libreport.8/src/include/problem_data.h      2012-07-20 14:03:06.891337223 
+0200
+++ libreport.9/src/include/problem_data.h      2012-07-23 18:48:46.990704360 
+0200
@@ -98,8 +98,6 @@ problem_data_t *create_problem_data_for_

 struct dump_dir *create_dump_dir_from_problem_data(problem_data_t 
*problem_data, const char *base_dir_name);

-char* save_dump_dir_from_problem_data(problem_data_t *problem_data, const char 
*base_dir_name);
-
 #ifdef __cplusplus
 }
 #endif
diff -x '*.po' -d -urpN libreport.8/src/lib/create_dump_dir.c 
libreport.9/src/lib/create_dump_dir.c
--- libreport.8/src/lib/create_dump_dir.c       2012-07-20 15:22:57.243932962 
+0200
+++ libreport.9/src/lib/create_dump_dir.c       2012-07-23 18:48:40.480705400 
+0200
@@ -31,16 +31,25 @@ static struct dump_dir *try_dd_create(co

 struct dump_dir *create_dump_dir_from_problem_data(problem_data_t 
*problem_data, const char *base_dir_name)
 {
-    char dir_name[sizeof("abrt-tmp-"LIBREPORT_ISO_DATE_STRING_SAMPLE"-%lu") + 
sizeof(long)*3];
-    snprintf(dir_name, sizeof(dir_name), "abrt-tmp-%s-%lu", 
iso_date_string(NULL), (long)getpid());
+    char *type = problem_data_get_content_or_NULL(problem_data, 
FILENAME_ANALYZER);
+
+    if (!type)
+    {
+        error_msg(_("Missing required item: '%s'"), FILENAME_ANALYZER);
+        return NULL;
+    }
+
+    char *problem_id = xasprintf("%s-%s-%lu"NEW_PD_SUFFIX, type, 
iso_date_string(NULL), (long)getpid());
+
+    VERB2 log("Saving to %s/%s", base_dir_name, problem_id);

     struct dump_dir *dd;
     if (base_dir_name)
-        dd = try_dd_create(base_dir_name, dir_name);
+        dd = try_dd_create(base_dir_name, problem_id);
     else
     {
         /* Try /var/run/abrt */
-        dd = try_dd_create(LOCALSTATEDIR"/run/abrt", dir_name);
+        dd = try_dd_create(LOCALSTATEDIR"/run/abrt", problem_id);
         /* Try $HOME/tmp */
         if (!dd)
         {
@@ -49,78 +58,18 @@ struct dump_dir *create_dump_dir_from_pr
             {
                 home = concat_path_file(home, "tmp");
                 /*mkdir(home, 0777); - do we want this? */
-                dd = try_dd_create(home, dir_name);
+                dd = try_dd_create(home, problem_id);
                 free(home);
             }
         }
 //TODO: try user's home dir obtained by getpwuid(getuid())?
         /* Try /tmp */
         if (!dd)
-            dd = try_dd_create("/tmp", dir_name);
-    }
-    if (!dd)
-        return NULL;
-
-    GHashTableIter iter;
-    char *name;
-    struct problem_item *value;
-    g_hash_table_iter_init(&iter, problem_data);
-    while (g_hash_table_iter_next(&iter, (void**)&name, (void**)&value))
-    {
-        if (name[0] == '.' || strchr(name, '/'))
-        {
-            error_msg("Problem data field name contains disallowed chars: 
'%s'", name);
-            goto next;
-        }
-
-//FIXME: what to do with CD_FLAG_BINs??
-        if (value->flags & CD_FLAG_BIN)
-            goto next;
-
-        dd_save_text(dd, name, value->content);
- next: ;
-    }
-
-    /* need to create basic files AFTER we save the pd to dump_dir
-     * otherwise we can't skip already created files like in case when
-     * reporting from anaconda where we can't read /etc/{system,redhat}-release
-     * and os_release is taken from anaconda
-    */
-    dd_create_basic_files(dd, (uid_t)-1L, NULL);
-
-    return dd;
-}
-
-char* save_dump_dir_from_problem_data(problem_data_t *problem_data, const char 
*base_dir_name)
-{
-    char *type = problem_data_get_content_or_NULL(problem_data, 
FILENAME_ANALYZER);
-
-    if (!type)
-    {
-        error_msg(_("Missing required item: '%s'"), FILENAME_ANALYZER);
-        return NULL;
-    }
-
-    char *time_s = problem_data_get_content_or_NULL(problem_data, 
FILENAME_TIME);
-    if (!time_s)
-    {
-        /* time is a required field, so if it's not provided add a default one 
*/
-        char buf[sizeof(unsigned long) * 3];
-        time_t t = time(NULL);
-        sprintf(buf, "%lu", (unsigned long)t);
-        problem_data_add_text_noteditable(problem_data, FILENAME_TIME, buf);
+            dd = try_dd_create("/tmp", problem_id);
     }

-    char *problem_id = xasprintf("%s-%s-%lu"NEW_PD_SUFFIX, type, 
iso_date_string(NULL), (long)getpid());
-
-    VERB2 log("Saving to %s/%s", base_dir_name, problem_id);
-    struct dump_dir *dd = try_dd_create(base_dir_name, problem_id);
-    if (!dd)
-    {
-        perror_msg("Can't create problem directory");
-        free(problem_id);
-        return NULL;
-    }
+    if (!dd) /* try_dd_create() already emitted the error message */
+        goto ret;

     GHashTableIter iter;
     char *name;
@@ -152,19 +101,19 @@ char* save_dump_dir_from_problem_data(pr
         dd_save_text(dd, name, value->content);
     }

-    problem_id[strlen(problem_id) - strlen(NEW_PD_SUFFIX)] = '\0';
+    /* need to create basic files AFTER we save the pd to dump_dir
+     * otherwise we can't skip already created files like in case when
+     * reporting from anaconda where we can't read /etc/{system,redhat}-release
+     * and os_release is taken from anaconda
+     */
+    dd_create_basic_files(dd, (uid_t)-1L, NULL);

+    problem_id[strlen(problem_id) - strlen(NEW_PD_SUFFIX)] = '\0';
     char* new_path = concat_path_file(base_dir_name, problem_id);
-
     VERB2 log("Renaming from '%s' to '%s'", dd->dd_dirname, new_path);
-    if (dd_rename(dd, new_path) != 0)
-    {
-        free(problem_id);
-        problem_id = NULL;
-    }
-
-    free(new_path);
-    dd_close(dd);
+    dd_rename(dd, new_path);

-    return problem_id;
+ ret:
+    free(problem_id);
+    return dd;
 }
diff -x '*.po' -d -urpN libreport.8/src/lib/dump_dir.c 
libreport.9/src/lib/dump_dir.c
--- libreport.8/src/lib/dump_dir.c      2012-07-10 18:07:42.000000000 +0200
+++ libreport.9/src/lib/dump_dir.c      2012-07-20 15:29:41.245932431 +0200
@@ -463,9 +463,15 @@ void dd_create_basic_files(struct dump_d
 {
     char long_str[sizeof(long) * 3 + 2];

-    time_t t = time(NULL);
-    snprintf(long_str, sizeof(long_str), "%lu", (long)t);
-    dd_save_text(dd, FILENAME_TIME, long_str);
+    char *time_str = dd_load_text_ext(dd, FILENAME_TIME,
+                    DD_FAIL_QUIETLY_ENOENT | 
DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);
+    if (!time_str)
+    {
+        time_t t = time(NULL);
+        sprintf(long_str, "%lu", (long)t);
+        dd_save_text(dd, FILENAME_TIME, long_str);
+    }
+    free(time_str);

     /* it doesn't make sense to create the uid file if uid == -1 */
     if (uid != (uid_t)-1L)
@@ -484,7 +490,7 @@ void dd_create_basic_files(struct dump_d
      * if it doesn't
      * i.e: anaconda doesn't have /etc/{fedora,redhat}-release and trying to 
load it
      * results in errors: rhbz#725857
-    */
+     */
     char *release = dd_load_text_ext(dd, FILENAME_OS_RELEASE,
                     DD_FAIL_QUIETLY_ENOENT | 
DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);

Reply via email to