Pushed the following ti git, please review.
The key moments are:
* Encapsulated event_option_t and event_config_t creation/deletion:
event_option_t *new_event_option(void);
void free_event_option(event_option_t *p);
event_config_t *new_event_config(void);
void free_event_config(event_config_t *p);
* Plugged event_config_t->malloced_member memory leaks in show_event_config_dialog()
* Fixed memory leaks in src/lib/event_xml_parser.c
--
vda
diff -x '*.po' -d -urpN abrt.4/src/gui-gtk/event_config_dialog.c abrt.5/src/gui-gtk/event_config_dialog.c
--- abrt.4/src/gui-gtk/event_config_dialog.c 2011-03-08 12:59:08.000000000 +0100
+++ abrt.5/src/gui-gtk/event_config_dialog.c 2011-03-08 16:11:32.221388055 +0100
@@ -84,11 +84,11 @@ void print_option(gpointer data, gpointe
void show_event_config_dialog(const char* event_name)
{
- event_config_t ui;
- ui.options = NULL;
- load_event_description_from_file(&ui, "Bugzilla.xml");
+ event_config_t *ui = new_event_config();
+ load_event_description_from_file(ui, "Bugzilla.xml");
+
GtkWidget *dialog = gtk_dialog_new_with_buttons(
- ui.name,
+ ui->name,
NULL,
GTK_DIALOG_MODAL | GTK_DIALOG_DESTROY_WITH_PARENT,
GTK_STOCK_OK,
@@ -96,10 +96,12 @@ void show_event_config_dialog(const char
GTK_STOCK_CANCEL,
GTK_RESPONSE_CANCEL,
NULL);
- int length = g_list_length(ui.options);
+ int length = g_list_length(ui->options);
//g_print("%i\n", length);
option_table = gtk_table_new(length, 2, 0);
- g_list_foreach(ui.options, &print_option, NULL);
+ g_list_foreach(ui->options, &print_option, NULL);
+ free_event_config(ui);
+
GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(dialog));
gtk_box_pack_start(GTK_BOX(content), option_table, 0, 0, 10);
gtk_widget_show_all(option_table);
diff -x '*.po' -d -urpN abrt.4/src/hooks/abrt-hook-ccpp.c abrt.5/src/hooks/abrt-hook-ccpp.c
--- abrt.4/src/hooks/abrt-hook-ccpp.c 2011-03-08 12:59:08.000000000 +0100
+++ abrt.5/src/hooks/abrt-hook-ccpp.c 2011-03-08 15:10:40.349158470 +0100
@@ -380,7 +380,7 @@ int main(int argc, char** argv)
unsigned setting_MaxCrashReportsSize = 0;
bool setting_MakeCompatCore = false;
bool setting_SaveBinaryImage = false;
- parse_conf(CONF_DIR"/plugins/CCpp.conf", &setting_MaxCrashReportsSize, &setting_MakeCompatCore, &setting_SaveBinaryImage);
+ parse_conf(PLUGINS_CONF_DIR"/CCpp.conf", &setting_MaxCrashReportsSize, &setting_MakeCompatCore, &setting_SaveBinaryImage);
if (!setting_SaveBinaryImage && src_fd_binary >= 0)
{
close(src_fd_binary);
diff -x '*.po' -d -urpN abrt.4/src/hooks/Makefile.am abrt.5/src/hooks/Makefile.am
--- abrt.4/src/hooks/Makefile.am 2011-03-08 12:59:08.000000000 +0100
+++ abrt.5/src/hooks/Makefile.am 2011-03-08 15:10:18.568408235 +0100
@@ -9,6 +9,7 @@ abrt_hook_ccpp_CPPFLAGS = \
-DDEBUG_DUMPS_DIR="$(DEBUG_DUMPS_DIR)" \
-DCONF_DIR="$(CONF_DIR)" \
-DVAR_RUN="$(VAR_RUN)" \
+ -DPLUGINS_CONF_DIR="$(PLUGINS_CONF_DIR)" \
$(GLIB_CFLAGS) \
-Wall \
-D_GNU_SOURCE
diff -x '*.po' -d -urpN abrt.4/src/include/report/event_config.h abrt.5/src/include/report/event_config.h
--- abrt.4/src/include/report/event_config.h 2011-03-08 12:59:08.000000000 +0100
+++ abrt.5/src/include/report/event_config.h 2011-03-08 15:57:24.092158509 +0100
@@ -52,6 +52,9 @@ typedef struct
int required;
} event_option_t;
+event_option_t *new_event_option(void);
+void free_event_option(event_option_t *p);
+
//structure to hold the option data
typedef struct
{
@@ -61,6 +64,10 @@ typedef struct
GList *options;
} event_config_t;
+event_config_t *new_event_config(void);
+void free_event_config(event_config_t *p);
+
+
void load_event_description_from_file(event_config_t *event_config, const char* filename);
// (Re)loads data from /etc/abrt/events/*.{conf,xml}
diff -x '*.po' -d -urpN abrt.4/src/lib/event_config.c abrt.5/src/lib/event_config.c
--- abrt.4/src/lib/event_config.c 2011-03-08 12:59:08.000000000 +0100
+++ abrt.5/src/lib/event_config.c 2011-03-08 16:17:48.220408362 +0100
@@ -1,17 +1,85 @@
-#include "event_config.h"
+#include "abrtlib.h"
GHashTable *g_event_config_list;
+event_option_t *new_event_option(void)
+{
+ return xzalloc(sizeof(event_option_t));
+}
+
+event_config_t *new_event_config(void)
+{
+ return xzalloc(sizeof(event_config_t));
+}
+
+void free_event_option(event_option_t *p)
+{
+ if (!p)
+ return;
+ free(p->name);
+ free(p->value);
+ free(p->label);
+ free(p->description);
+ free(p->allowed_value);
+ free(p);
+}
+
+void free_event_config(event_config_t *p)
+{
+ if (!p)
+ return;
+ free(p->name);
+ free(p->title);
+ free(p->action);
+ for (GList *opt = p->options; opt; opt = opt->next)
+ free_event_option(opt->data);
+ g_list_free(p->options);
+ free(p);
+}
+
+
// (Re)loads data from /etc/abrt/events/*.{conf,xml}
void load_event_config_data(void)
{
free_event_config_data();
- /* for each xml file call load_event_description_from_file */
- /* for each conf file call load_even_options_value_from_file?
- * - we don't have this
- * - should re-use the event_config structure created when parsing xml - if exists
- * - or should this one be called first?
- */
+
+ DIR *dir = opendir(EVENTS_DIR);
+ if (!dir)
+ return;
+
+ if (!g_event_config_list)
+ g_event_config_list = g_hash_table_new_full(
+ /*hash_func*/ g_str_hash,
+ /*key_equal_func:*/ g_str_equal,
+ /*key_destroy_func:*/ free,
+ /*value_destroy_func:*/ (GDestroyNotify) free_event_config
+ );
+
+ struct dirent *dent;
+ while ((dent = readdir(dir)) != NULL)
+ {
+ char *ext = strrchr(dent->d_name, '.');
+ if (!ext)
+ continue;
+ ext++;
+ bool conf = strcmp(ext, "conf") == 0;
+ bool xml = strcmp(ext, "xml") == 0;
+ if (!conf && !xml)
+ continue;
+
+ event_config_t *event_config = new_event_config();
+
+ char *fullname = concat_path_file(EVENTS_DIR, dent->d_name);
+ if (xml)
+ load_event_description_from_file(event_config, fullname);
+// if (conf)
+// load_event_values_from_file(event_config, fullname);
+
+ free(fullname);
+
+ *ext = '\0';
+ g_hash_table_replace(g_event_config_list, xstrdup(dent->d_name), event_config);
+ }
}
/* Frees all loaded data */
diff -x '*.po' -d -urpN abrt.4/src/lib/event_xml_parser.c abrt.5/src/lib/event_xml_parser.c
--- abrt.4/src/lib/event_xml_parser.c 2011-03-08 14:55:16.000000000 +0100
+++ abrt.5/src/lib/event_xml_parser.c 2011-03-08 16:21:30.590158378 +0100
@@ -1,7 +1,3 @@
-//#include <glib.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
#include "abrtlib.h"
#include "event_config.h"
@@ -13,7 +9,7 @@
#define ACTION_ELEMENT "action"
#define NAME_ELEMENT "name"
-int in_option = 0;
+static int in_option = 0; //FIXME
static const char *const option_types[] =
{
@@ -34,30 +30,33 @@ static void start_element(GMarkupParseCo
{
//g_print("start: %s\n", element_name);
- event_config_t *ui = (event_config_t *)user_data;
+ event_config_t *ui = user_data;
- if(strcmp(element_name, OPTION_ELEMENT) == 0)
+ if (strcmp(element_name, OPTION_ELEMENT) == 0)
{
- if(in_option == 0)
+ if (in_option == 0)
{
in_option = 1;
- event_option_t *option = (event_option_t *)malloc(sizeof(event_option_t));
+ event_option_t *option = xzalloc(sizeof(*option));
//we need to prepend, so ui->options always points to the last created option
VERB2 log("adding option");
ui->options = g_list_prepend(ui->options, option);
int i;
- for(i = 0; attribute_names[i] != NULL; ++i)
+ for (i = 0; attribute_names[i] != NULL; ++i)
{
- VERB2 log("attr: %s:%s\n", attribute_names[i], attribute_values[i]);
- if(strcmp(attribute_names[i], "name") == 0)
- option->name = strdup(attribute_values[i]);
- if(strcmp(attribute_names[i], "type") == 0)
+ VERB2 log("attr: %s:%s", attribute_names[i], attribute_values[i]);
+ if (strcmp(attribute_names[i], "name") == 0)
{
- option_type_t type = 0;
- for(type = OPTION_TYPE_TEXT; type < OPTION_TYPE_INVALID; ++type)
+ free(option->name);
+ option->name = xstrdup(attribute_values[i]);
+ }
+ else if (strcmp(attribute_names[i], "type") == 0)
+ {
+ option_type_t type;
+ for (type = OPTION_TYPE_TEXT; type < OPTION_TYPE_INVALID; ++type)
{
- if(strcmp(option_types[type], attribute_values[i]) == 0)
+ if (strcmp(option_types[type], attribute_values[i]) == 0)
option->type = type;
}
}
@@ -65,74 +64,79 @@ static void start_element(GMarkupParseCo
}
else
{
- g_print("error, option nested in option!\n");
+ error_msg("error, option nested in option");
}
}
}
- // Called for close tags </foo>
+// Called for close tags </foo>
static void end_element(GMarkupParseContext *context,
const gchar *element_name,
gpointer user_data,
GError **error)
{
- event_config_t *ui = (event_config_t *)user_data;
- if(strcmp(element_name, OPTION_ELEMENT) == 0)
+ event_config_t *ui = user_data;
+ if (strcmp(element_name, OPTION_ELEMENT) == 0)
{
in_option = 0;
}
- if(strcmp(element_name, EVENT_ELEMENT) == 0)
+ if (strcmp(element_name, EVENT_ELEMENT) == 0)
{
- //we need to revers the list, because we we're prepending
+ //we need to reverse the list, because we we're prepending
ui->options = g_list_reverse(ui->options);
in_option = 0;
}
}
- // Called for character data
- // text is not nul-terminated
+// Called for character data
+// text is not nul-terminated
static void text(GMarkupParseContext *context,
const gchar *text,
gsize text_len,
gpointer user_data,
GError **error)
{
- event_config_t *ui = (event_config_t *)user_data;
+ event_config_t *ui = user_data;
const gchar * inner_element = g_markup_parse_context_get_element(context);
- char *_text = (char *)malloc(text_len+1);
- strncpy(_text, text, text_len);
- _text[text_len] = '\0';
- if(in_option == 1)
+ char *_text = xstrndup(text, text_len);
+ if (in_option == 1)
{
- event_option_t *option = (event_option_t *)((ui->options)->data);
- if(strcmp(inner_element, LABEL_ELEMENT) == 0)
+ event_option_t *option = ui->options->data;
+ if (strcmp(inner_element, LABEL_ELEMENT) == 0)
{
- VERB2 log("\tnew label: \t\t%s", _text);
+ VERB2 log("new label:'%s'", _text);
+ free(option->label);
option->label = _text;
+ return;
}
- if(strcmp(inner_element, DESCRIPTION_ELEMENT) == 0)
+ if (strcmp(inner_element, DESCRIPTION_ELEMENT) == 0)
{
- VERB2 log("\ttooltip: \t\t%s", _text);
+ VERB2 log("tooltip:'%s'", _text);
+ free(option->description);
option->description = _text;
+ return;
}
}
else
{
/* we're not in option, so the description is for the event */
- if(strcmp(inner_element, ACTION_ELEMENT) == 0)
+ if (strcmp(inner_element, ACTION_ELEMENT) == 0)
{
- VERB2 log("\taction description: \t%s", _text);
+ VERB2 log("action description:'%s'", _text);
+ free(ui->action);
ui->action = _text;
+ return;
}
- if(strcmp(inner_element, NAME_ELEMENT) == 0)
+ if (strcmp(inner_element, NAME_ELEMENT) == 0)
{
- VERB2 log("\tevent name: \t\t%s", _text);
+ VERB2 log("event name:'%s'", _text);
+ free(ui->name);
ui->name = _text;
+ return;
}
}
- //will be freed when the event_option is freed
- //free(_text);
+ free(_text);
}
// Called for strings that should be re-saved verbatim in this same
@@ -154,12 +158,12 @@ static void error(GMarkupParseContext *c
GError *error,
gpointer user_data)
{
- g_print("error\n");
+ error_msg("error in XML parsing");
}
/* this function takes 2 parameters
- * ui -> pointer to event_config_t
- * filename -> filename to read
+ * ui -> pointer to event_config_t
+ * filename -> filename to read
* event_config_t contains list of options, which is malloced by hits function
* and must be freed by the caller
*/
@@ -174,14 +178,16 @@ void load_event_description_from_file(ev
parser.error = &error;
GMarkupParseContext *context = g_markup_parse_context_new(
&parser, G_MARKUP_TREAT_CDATA_AS_TEXT,
- event_config, NULL);
+ event_config, /*GDestroyNotify:*/ NULL);
+
FILE* fin = fopen(filename, "r");
size_t read_bytes = 0;
char buff[1024];
- while((read_bytes = fread(buff, 1, 1024, fin)))
+ while ((read_bytes = fread(buff, 1, 1024, fin)))
{
g_markup_parse_context_parse(context, buff, read_bytes, NULL);
}
fclose(fin);
+
g_markup_parse_context_free(context);
}
diff -x '*.po' -d -urpN abrt.4/src/lib/Makefile.am abrt.5/src/lib/Makefile.am
--- abrt.4/src/lib/Makefile.am 2011-03-08 12:59:08.000000000 +0100
+++ abrt.5/src/lib/Makefile.am 2011-03-08 15:17:41.406408649 +0100
@@ -49,12 +49,13 @@ libreport_la_SOURCES = \
libreport_la_CPPFLAGS = \
-Wall -Werror \
-I$(srcdir)/../include/report -I$(srcdir)/../include \
+ -DLOCALSTATEDIR='"$(localstatedir)"' \
+ -DVAR_RUN="$(VAR_RUN)" \
-DDEBUG_DUMPS_DIR="$(DEBUG_DUMPS_DIR)" \
-DPLUGINS_LIB_DIR="$(PLUGINS_LIB_DIR)" \
-DPLUGINS_CONF_DIR="$(PLUGINS_CONF_DIR)" \
- -DLOCALSTATEDIR='"$(localstatedir)"' \
-DCONF_DIR="$(CONF_DIR)" \
- -DVAR_RUN="$(VAR_RUN)" \
+ -DEVENTS_DIR="$(EVENTS_DIR)" \
$(GLIB_CFLAGS) \
-D_GNU_SOURCE
libreport_la_LDFLAGS = \
@@ -66,11 +67,13 @@ libabrt_dbus_la_SOURCES = \
abrt_dbus.c abrt_dbus.h
libabrt_dbus_la_CPPFLAGS = \
-I$(srcdir)/../include/report -I$(srcdir)/../include \
+ -DLOCALSTATEDIR='"$(localstatedir)"' \
+ -DVAR_RUN="$(VAR_RUN)" \
-DDEBUG_DUMPS_DIR="$(DEBUG_DUMPS_DIR)" \
-DPLUGINS_LIB_DIR="$(PLUGINS_LIB_DIR)" \
-DPLUGINS_CONF_DIR="$(PLUGINS_CONF_DIR)" \
-DCONF_DIR="$(CONF_DIR)" \
- -DVAR_RUN="$(VAR_RUN)" \
+ -DEVENTS_DIR="$(EVENTS_DIR)" \
$(GLIB_CFLAGS) \
$(DBUS_CFLAGS) \
-Wall -Werror \
@@ -87,11 +90,13 @@ libabrt_web_la_SOURCES = \
libabrt_web_la_CPPFLAGS = \
-Wall -Werror \
-I$(srcdir)/../include/report -I$(srcdir)/../include \
+ -DLOCALSTATEDIR='"$(localstatedir)"' \
+ -DVAR_RUN="$(VAR_RUN)" \
-DDEBUG_DUMPS_DIR="$(DEBUG_DUMPS_DIR)" \
-DPLUGINS_LIB_DIR="$(PLUGINS_LIB_DIR)" \
-DPLUGINS_CONF_DIR="$(PLUGINS_CONF_DIR)" \
-DCONF_DIR="$(CONF_DIR)" \
- -DVAR_RUN="$(VAR_RUN)" \
+ -DEVENTS_DIR="$(EVENTS_DIR)" \
$(GLIB_CFLAGS) \
$(CURL_CFLAGS) \
$(LIBXML_CFLAGS) \