[Bug 24257] TpAccount, TpAccountManager: prepare never terminates if the object is invalidated

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=24257





--- Comment #11 from Simon McVittie <simon.mcvittie at collabora.co.uk>  2009-11-11 04:09:49 PST ---
(In reply to comment #10)
> "fd.o#24257: tp_account_prepare: fail if invalidated" is clearer when truncated
> to 50 chars.

Given that it's often difficult to write a good one-line commit message in *80*
characters, I don't think truncating to 50 should necessarily be a concern :-)

> I'm slightly confused by the difference between return.*if_fail and assert.
> They are both used for programmer errors, and both cause the program to abort
> when I use them. I suppose they do different things if you turn asserts off or
> something?

g_return(_val)_if_fail: the caller of this function was wrong

g_assert: the internal state of this code is wrong

The failure of a g_return(_val)_if_fail is a g_critical (critical warning, not
fatal by default) plus an early return. The caller might crash (e.g. presumably
they weren't expecting NULL to be returned, certainly not with the GError
unset) but that's their problem.

The failure of a g_assert is a g_error, which is always fatal.

Most Telepathy regression tests, and most of our CMs, run with fatal criticals
in order to promote high-quality code.

> Changed to g_return_val_if_fail (account != NULL, NULL); and changed the commit
> message to reflect this. Tell me if you want 
> g_return_val_if_fail (tp_account_parse_object_path (object_path, NULL, NULL,
> NULL, error), NULL); as well.

I think that's reasonable.

> > <http://git.collabora.co.uk/?p=user/alsuren/telepathy-glib.git;a=commitdiff;h=8ab93a727864f0117c08b789cc62a438e4adf15f>:
> > > +  g_assert (test->prepared == tp_account_manager_is_prepared (test->am, TP_ACCOUNT_MANAGER_FEATURE_CORE));
> > 
> > g_assert_intcmp (t_a_m_i_p (), ==, t->p) ?
> > 
> Fair point. Actually, should I be putting !! in front of bools before comparing
> them, or is gboolean guaranteed to be 1 or 0 somehow?

gboolean is secretly a gint, so yes it can have non-1 true values. In
production code you should never compare gbooleans by equality, but I think in
tests it's acceptable to do so - in a test, you want your assertion to be
strict, after all.


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