On 12/03/2010 07:27 PM, Denys Vlasenko wrote:
This patch changes crash data to use C structures.
The smallest data element is:
struct crash_item {
char *content;
unsigned flags;
};
where content is, eh, content, and flags is a bit flag field:
enum {
CD_FLAG_SYS = (1<< 0),
CD_FLAG_BIN = (1<< 1),
CD_FLAG_TXT = (1<< 2),
CD_FLAG_ISEDITABLE = (1<< 3),
CD_FLAG_ISNOTEDITABLE = (1<< 4),
};
crash_data_t is a map of crash_item's, implemented as a pointer
to heap-allocated GHashTable.
vector_of_crash_data_t is a vector of crash_data_t's, implemented
as a pointer to heap-allocated GPtrArray.
Most operations have light wrappers around them to hide the nature
of the containers. For example, to free vector_of_crash_data,
you need to use free_vector_of_crash_data(ptr) instead of
open-coding g_ptr_array_free. The wrapper is thin:
static inline void free_vector_of_crash_data(vector_of_crash_data_t *vector)
{
if (vector)
g_ptr_array_free(vector, TRUE);
}
The goal is not so much to hide the implementation, but more
to make it easier to use the correct function.
dbus (un)marshalling functions convert crash_item to three-element
array of strings, in order to keep compatibility with abrt-gui (python).
This can be changed later to use native representation.
crash_data_t and vector_of_crash_data_t are represented in
"natural" way, no funny stuff there.
Lightly run-tested.
Please review.
make sure you don't have any crash in /var/spool/abrt
run abrtd -dvvv
$ abrt-cli -r @0
abrtd daemon:
abrtd: CreateCrashReport('@0',500)
abrtd: '@0' does not exist
abrtd: Warning(':1.129'): '@0' does not exist
abrtd: DBus message sent
abrtd: Can't find crash with id '@0'
abrtd: Warning(':1.129'): Can't find crash with id '@0'
abrtd: DBus message sent
(process:26506): GLib-CRITICAL **: g_hash_table_iter_init: assertion
`hash_table != NULL' failed
(process:26506): GLib-CRITICAL **: g_hash_table_iter_next: assertion
`ri->version == ri->hash_table->version' failed
also when you run abrt-cli --info @0
abrtd: CreateCrashReport('@0',500)
abrtd: '@0' does not exist
abrtd: Warning(':1.147'): '@0' does not exist
abrtd: DBus message sent
abrtd: Can't find crash with id '@0'
abrtd: Warning(':1.147'): Can't find crash with id '@0'
abrtd: DBus message sent
(process:26818): GLib-CRITICAL **: g_hash_table_iter_init: assertion
`hash_table != NULL' failed
+const char *get_crash_data_item_content(crash_data_t *crash_data, const
char *key)
+{
+ struct crash_item *item = get_crash_data_item_or_NULL(crash_data, key);
+ if (!item)
+ error_msg_and_die("Error accessing crash data: no ['%s']",
key);
+ return item->content;
+}
Firs I saw this function I thought that return char* or NULL, but I was
surprise that its die.
I propose better name like get_crash_data_item_content_or_die
and
+int load_vector_of_crash_data(DBusMessageIter* iter,
vector_of_crash_data_t **val)
+{
+ *val = NULL;
Here you have protection ahead of not clever people to pass . According
to me if someone
create variable and he/she is not init variable it's his/her fault that
he/she pass garbage from memory.
I would put there only assert.