On Tuesday 26 of June 2012 12:34:46 Jiri Moskovcak wrote:
- should be used in call from hook api, maybe it can even replace the former create_dump_dir_from_problem_data
po/POTFILES.in | 1 + src/include/problem_data.h | 2 + src/include/report.h | 8 ++++ src/lib/create_dump_dir.c | 94 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+)
diff --git a/po/POTFILES.in b/po/POTFILES.in index 3203183..09ff83e 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -9,6 +9,7 @@ src/gtk-helpers/event_config_dialog.c src/gui-wizard-gtk/main.c src/gui-wizard-gtk/wizard.c src/gui-wizard-gtk/wizard.glade +src/lib/create_dump_dir.c src/lib/event_config.c src/lib/parse_options.c src/lib/curl.c diff --git a/src/include/problem_data.h b/src/include/problem_data.h index 9bf46f5..c90f4a1 100644 --- a/src/include/problem_data.h +++ b/src/include/problem_data.h @@ -35,6 +35,7 @@ enum { /* Show this element in "short" info (report-cli -l) */ CD_FLAG_LIST = (1 << 4), CD_FLAG_UNIXTIME = (1 << 5),
- CD_FLAG_FILE = (1 << 6),
};
struct problem_item { @@ -61,6 +62,7 @@ void add_basics_to_problem_data(problem_data_t *pd);
static inline void free_problem_data(problem_data_t *problem_data) {
- //TODO: leaks problem item; if (problem_data) g_hash_table_destroy(problem_data);
} diff --git a/src/include/report.h b/src/include/report.h index 3735ef6..21557ac 100644 --- a/src/include/report.h +++ b/src/include/report.h @@ -39,6 +39,12 @@ enum { LIBREPORT_RUN_NEWT = (1 << 8), /* run 'report-newt' */ };
+typedef enum {
- LR_OK = 0,
- LR_MISSING_ITEM = (1 << 1),
- LR_ERROR = (1 << 2),
+} LibreportError;
int report_problem_in_dir(const char *dirname, int flags);
/* Reports a problem stored in problem_data_t. @@ -49,6 +55,8 @@ int report_problem_in_memory(problem_data_t *pd, int flags); /* Simple wrapper for trivial uses */ int report_problem(problem_data_t *pd);
+LibreportError save_dump_dir_from_problem_data(problem_data_t *problem_data, char **problem_id, const char *base_dir_name); + #ifdef __cplusplus } #endif diff --git a/src/lib/create_dump_dir.c b/src/lib/create_dump_dir.c index 137da12..a089a3b 100644 --- a/src/lib/create_dump_dir.c +++ b/src/lib/create_dump_dir.c @@ -19,6 +19,8 @@
#include "internal_libreport.h"
+#define NEW_PD_SUFFIX ".new"
static struct dump_dir *try_dd_create(const char *base_dir_name, const char *dir_name) { char *path = concat_path_file(base_dir_name, dir_name); @@ -88,3 +90,95 @@ struct dump_dir *create_dump_dir_from_problem_data(problem_data_t *problem_data,
return dd;
}
+LibreportError save_dump_dir_from_problem_data(problem_data_t *problem_data, char **problem_id, const char *base_dir_name) +{
- char *type = get_problem_item_content_or_NULL(problem_data, "type");
Shouldn't we use FILENAME_TYPE instead of a string literal?
- if(!type)
- {
error_msg(_("Missing required item: 'type'"));
return LR_MISSING_ITEM;
- }
- char *time_s = get_problem_item_content_or_NULL(problem_data, "time");
Shouldn't we use FILENAME_TIME instead of a string literal?
- if(!time_s)
- {
/* 64 is a randomly picked constant which should be
* enough to hold the time in seconds for a long time..
*/
char buf[64];
You known that you are casting the t variable to long unsigned, so why not to set buf's size to sizeof(long unsigned)*3 ?
time_t t = time(NULL);
/* time is a required field, so if it's not provided add a default
one */ + snprintf(buf, sizeof(buf), "%lu", (long unsigned)t);
add_to_problem_data_ext(problem_data, "time", buf, CD_FLAG_TXT);
- }
- char dir_name[strlen(type) +
sizeof("-"LIBREPORT_ISO_DATE_STRING_SAMPLE"-%lu") + sizeof(long)*3]; + *problem_id = (char *)xmalloc(sizeof(dir_name));
- snprintf(*problem_id, sizeof(dir_name), "%s-%s-%lu"NEW_PD_SUFFIX, type,
iso_date_string(NULL), (long)getpid()); + strncpy(dir_name, *problem_id, sizeof(dir_name));
I guess, xasprintf() could be used here. It is not necessary to use an extra variable dir_name. (please, see line 167)
- VERB2 log("Saving to %s/%s", base_dir_name, *problem_id);
- struct dump_dir *dd = try_dd_create(base_dir_name, dir_name);
- if (!dd)
- {
perror_msg("Can't create problem directory");
*problem_id = NULL;
Leaking problem_id
return LR_ERROR;
- }
- 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 (value->flags & CD_FLAG_FILE)
{
char dest[PATH_MAX+1];
snprintf(dest, sizeof(dest), "%s/%s", dd->dd_dirname, name);
VERB2 log("copying '%s' to '%s'", value->content, dest);
off_t copied = copy_file(value->content, dest, 0644);
if (copied < 0)
error_msg("Can't copy %s to %s", value->content, dest);
VERB2 log("copied %li bytes", (unsigned long)copied);
continue;
}
/* only files should contain '/' and those are handled earlier */
if (name[0] == '.' || strchr(name, '/'))
{
error_msg("Problem data field name contains disallowed chars:
'%s'", name); + continue;
}
+//FIXME: what to do with CD_FLAG_BINs??
if (value->flags & CD_FLAG_BIN)
continue;
dd_save_text(dd, name, value->content);
- }
- char *suffix = strstr(*problem_id, NEW_PD_SUFFIX);
- if (suffix)
*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)
- {
*problem_id = NULL;
Leaking problem_id
return LR_ERROR;
- }
- free(new_path);
- dd_close(dd);
- return LR_OK;
+}
Regards, Jakub