[Bug 54198] Logger should read logs from superseded accounts as well

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=54198

Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nicolas.dufresne at collabora.
                   |                            |co.uk

--- Comment #2 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2012-08-29 15:31:57 UTC ---
>-  const char *account_name = get_account_name (account);
>+  const gchar *account_name = account_path +
>+      strlen (TP_ACCOUNT_OBJECT_PATH_BASE);

Please add an assert to check that account_path is long enough. (multiple
places)

>log_store_xml_exists (TplLogStore *store,

Ishh, that synchronous function becomes worst then ever, hopefully it won't
heart to much the UI performance.

>+          if (g_list_find_custom (ret, new->data,
>+                (GCompareFunc) g_date_compare))
>+            g_date_free (new->data);
>+          else
>+            ret = g_list_insert_sorted (ret, new->data,
>+                  (GCompareFunc) g_date_compare);

We have had serious performance issues in the past. In this case there is
faster solution, like inserting everything once, and removing the duplicates in
1 pass. Second option is to implement an _insert_sorted_and_unique() method in
util.c, so it can be reused all over. This one is probably the optimal
solution.

>+          ret = g_list_insert_sorted (ret, new->data,
>+                (GCompareFunc) _tpl_event_timestamp_compare);

Use of insert sorted with events is wrong as it reverts the order of event with
equal timestamps. See util.h for an insert_after, you'll have to use queues.

Finally, this won't go through without rigorous unit tests. Thinking of it, I'm
not sure that superseded account API really works. It seems you can break it by
creating a second time the same account and removing the old one. The resulting
supersedes will not match.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
You are the assignee for the bug.