Bug 30046

Summary:	Invalid accounts (missing managers or similar major defects) can cause SEGVs
Product:	Telepathy	Reporter:	Vivek Dasmohapatra <vivek>
Component:	mission-control	Assignee:	Vivek Dasmohapatra <vivek>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	Keywords:	patch
Version:	git master
Hardware:	All
OS:	All
URL:	http://git.collabora.co.uk/?p=user/vivek/telepathy-mission-control;a=shortlog;h=refs/heads/invalid-account-cleanups
Whiteboard:	review+
i915 platform:		i915 features:

Description Vivek Dasmohapatra 2010-09-06 09:23:33 UTC

Accounts which are so badly broken (missing managers is one case) that they
can't be fully processed can still get onto the account list, and don't
get removed properly - this leaves accounts with invalid/uncleared 
bus name pointers (and other problems) in the invalid account list, ready
to explode the next time someone fetches the invalid account list.

Accounts that are this badly broken should never get onto the account list
at all.

Comment 1 Simon McVittie 2010-09-06 09:47:21 UTC

> +#define FORGET_POINTER(_ptr) g_free (_ptr); _ptr = NULL;

Replacing each call to FORGET_POINTER with tp_clear_pointer (&thing, g_free) would be better (requires tp-glib 0.11.7).

> +        plausible = (manager != NULL && *manager != '\0' &&
> +                     protocol != NULL && *protocol != '\0');

Two calls to tp_str_empty() would be exactly equivalent and more human-readable.

> +            g_warning ("%s: account %s has implausible manager/protocol: %s/%s",
> +                       G_STRFUNC, *name, manager, protocol);

This can segfault on platforms whose sprintf can't deal with NULL. We aim to be portable to non-glibc.

Otherwise, this looks good.

Comment 2 Vivek Dasmohapatra 2010-09-06 11:16:55 UTC

(In reply to comment #1)

> Replacing each call to FORGET_POINTER with tp_clear_pointer (&thing, g_free)
> would be better (requires tp-glib 0.11.7).

Done.

> Two calls to tp_str_empty() would be exactly equivalent and more
> human-readable.

Done

> This can segfault on platforms whose sprintf can't deal with NULL. We aim to be
> portable to non-glibc.

NULL pointer airbags added.

Comment 3 Simon McVittie 2010-09-07 04:46:54 UTC

r+

Comment 4 Vivek Dasmohapatra 2010-09-07 05:41:11 UTC

Merged to master.

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.