Summary: | Logs with weird strings may causes SEGV when reloaded | ||
---|---|---|---|
Product: | Telepathy | Reporter: | Daniel Thompson <daniel.thompson> |
Component: | logger | Assignee: | Telepathy bugs list <telepathy-bugs> |
Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
Severity: | normal | ||
Priority: | medium | CC: | nicolas |
Version: | 0.8 | ||
Hardware: | Other | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Attachments: |
logger: Better NULL checking in tpl_log_iter_xml_get_event
log-iter-xml: Fix crash when unable to parse a file tests: Add new TpLogger logs that don't parse as XML tests: Add a test for log-iter-xml/get-events tests: Add new TpLogger logs that don't parse as XML tests: Add a test for log-iter-xml/get-events log-iter-xml: Fix crash when logs are corrupted or invalid XML |
Description
Daniel Thompson
2015-03-16 14:47:31 UTC
Created attachment 114349 [details] [review] logger: Better NULL checking in tpl_log_iter_xml_get_event Comment on attachment 114349 [details] [review] logger: Better NULL checking in tpl_log_iter_xml_get_event Review of attachment 114349 [details] [review]: ----------------------------------------------------------------- Thanks for tracking this down, Daniel. I am not a telepathy reviewer, but I did write most of that file... It would be better if we tried to skip the date with the faulty file and tried the next one. Instead of completely bailing out. ::: telepathy-logger/log-iter-xml.c @@ +80,5 @@ > priv->events = _tpl_log_store_get_events_for_date (priv->store, > priv->account, priv->target, priv->type_mask, > (GDate *) priv->next_date->data); > + if (!priv->events) > + break; Nitpick: priv->events == NULL Created attachment 115213 [details] [review] log-iter-xml: Fix crash when unable to parse a file How about something like this? I try to test this if I can. Unfortunately the broken log files have been rotated out now so I no longer get the error when starting Polari. I'll see if I can corrupt them deliberately. Created attachment 115217 [details] [review] tests: Add new TpLogger logs that don't parse as XML Created attachment 115218 [details] [review] tests: Add a test for log-iter-xml/get-events Since bug 40675 , we try to recover from XML parsing failures as much as possible. That seems to be enough for all the faulty XML files that were reported for the original Polari bug [1]. Unfortunately, that patch isn't in the 0.8.x series, so users don't benefit from it. Even then, I guess, it doesn't hurt to defend against this case. [1] https://bugzilla.gnome.org/show_bug.cgi?id=720179 Created attachment 115219 [details] [review] tests: Add new TpLogger logs that don't parse as XML Created attachment 115220 [details] [review] tests: Add a test for log-iter-xml/get-events Created attachment 115221 [details] [review] log-iter-xml: Fix crash when logs are corrupted or invalid XML From #telepathy on Freenode: 16:44 <rishi> stormer: Are you ok with the pushing the patch that checks for NULL? 16:45 <stormer> rishi: yes 16:45 <rishi> ok 16:45 <stormer> definitly Pushed to both master and telepathy-logger-0.8 |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.