diff options
author | David Laban <david.laban@collabora.co.uk> | 2011-05-19 18:51:43 -0400 |
---|---|---|
committer | David Laban <david.laban@collabora.co.uk> | 2011-05-19 18:51:43 -0400 |
commit | 5b0525472dd226396e42939b1ff7aa10508fff43 (patch) | |
tree | f46e75ba38a7bbcba716d85d2ba826d67ed4fecc /logger | |
parent | d7a8928532fc85955e89f70d2398b3092c583ff9 (diff) |
fixup! log_store_xml_get_events_for_file: replace edited messages.
* update to private _tpl_text_event_add_supersedes
Also addresses:
> I think you don't have to call g_hash_table_insert() here ? And same in next
> for loop.
Actually, this is for the chained supersedes use-case, but yeah: it's wrong.
Fixed it when I implemented event_queue_replace_and_supersede (),
and fixed the comment on superseded_links
> Use g_list_previous().
> Is that suppose to be funny ?
(apparently it wasn't funny; fixed)
> Your leaking a reference on the dumm_event. Maybe you want to rework you
> _add_supersedes() method into something like link_supersedes() that would take
> ownership ?
Created method event_queue_replace_and_supersede with similar feel
to event_queue_insert_sorted_after.
This is something that I wanted to do, but couldn't see a clean way to do.
Fixing the semantics of superseded_links made this easier.
> You don't check anymore if parsing event worked. A corrupted XML file will
> cause your code to crash.
>
*_parse_*() will currently only return NULL if g_object_new() does, but you're
right: if someone changes this behaviour later, I should be resilient to it.
Fixed.
Diffstat (limited to 'logger')
-rw-r--r-- | logger/telepathy-logger/log-store-xml.c | 48 |
1 files changed, 31 insertions, 17 deletions
diff --git a/logger/telepathy-logger/log-store-xml.c b/logger/telepathy-logger/log-store-xml.c index 80f141903..40c709275 100644 --- a/logger/telepathy-logger/log-store-xml.c +++ b/logger/telepathy-logger/log-store-xml.c @@ -1267,6 +1267,19 @@ event_queue_insert_sorted_after (GQueue *events, } } +static void +event_queue_replace_and_supersede (GQueue *events, + GList *index, + GHashTable *superseded_links, + TplEvent *event) +{ + _tpl_text_event_add_supersedes (event, index->data); + g_hash_table_insert (superseded_links, + (gpointer) tpl_text_event_get_message_token(l->data), l); + g_object_unref (l->data); + l->data = event; +} + static GList * event_queue_add_text_event (GQueue *events, GList *index, @@ -1283,29 +1296,24 @@ event_queue_add_text_event (GQueue *events, l = g_hash_table_lookup (superseded_links, supersedes_token); if (l != NULL) { - tpl_text_event_add_supersedes (event, l->data); - g_object_unref (l->data); - l->data = event; - g_hash_table_insert (superseded_links, (gpointer) supersedes_token, l); + event_queue_replace_and_supersede (events, l, superseded_links, event); return index; } /* Search backwards from "now" and insert (but don't update "now") */ - for (l = index; l != NULL; l = l->prev) + for (l = index; l != NULL; l = g_list_previous (l)) { if (!tp_strdiff (tpl_text_event_get_message_token (l->data), supersedes_token)) { - tpl_text_event_add_supersedes (event, l->data); - g_object_unref (l->data); - l->data = event; - g_hash_table_insert (superseded_links, (gpointer) supersedes_token, l); + event_queue_replace_and_supersede (events, l, superseded_links, + event); return index; } } DEBUG ("Can't find event %s (superseded by %s). " - "Better <s>drink my own piss</s> fake it.", + "Adding Dummy event.", supersedes_token, tpl_text_event_get_message_token (event)); dummy_event = g_object_new (TPL_TYPE_TEXT_EVENT, @@ -1321,11 +1329,9 @@ event_queue_add_text_event (GQueue *events, "message-token", supersedes_token, NULL); - tpl_text_event_add_supersedes (event, dummy_event); index = event_queue_insert_sorted_after (events, index, - TPL_EVENT (event)); - g_hash_table_insert (superseded_links, (gpointer) supersedes_token, - index); + TPL_EVENT (dummy_event)); + event_queue_replace_and_supersede (events, index, superseded_links, event); return index; } @@ -1409,9 +1415,9 @@ log_store_xml_get_events_for_file (TplLogStoreXml *self, g_free (dirname); g_free (tmp); - /* Temporary hash from borrowed supersedes-token to borrowed link in - * events, so that we can efficiently replace repeatedly superseded - * events as their replacements come in. */ + /* Temporary hash from (borrowed) supersedes-token to (borrowed) link in + * events, for any event that was once in events, but has since been + * superseded (and therefore won't be found by a linear search). */ supersedes_links = g_hash_table_new (g_str_hash, g_str_equal); /* Now get the events. */ @@ -1425,6 +1431,10 @@ log_store_xml_get_events_for_file (TplLogStoreXml *self, { event = parse_text_node (self, node, is_room, target_id, self_id, account); + + if (event == NULL) + continue; + index = event_queue_add_text_event (events, index, supersedes_links, TPL_TEXT_EVENT (event)); num_events++; @@ -1434,6 +1444,10 @@ log_store_xml_get_events_for_file (TplLogStoreXml *self, { event = parse_call_node (self, node, is_room, target_id, self_id, account); + + if (event == NULL) + continue; + index = event_queue_insert_sorted_after (events, index, event); num_events++; } |