libzeitgeist

Merge lp://staging/~mhr3/libzeitgeist/fix-986230 into lp://staging/libzeitgeist

fix-986230
Merge into trunk

Proposed by Michal Hruby on 2012-04-27

Status:	Merged
Approved by:	Mikkel Kamstrup Erlandsen on 2012-04-27
Approved revision:	238
Merged at revision:	238
Proposed branch:	lp://staging/~mhr3/libzeitgeist/fix-986230
Merge into:	lp://staging/libzeitgeist
Diff against target:	219 lines (+80/-10) 2 files modified configure.ac (+1/-1) src/zeitgeist-log.c (+79/-9)
To merge this branch:	bzr merge lp://staging/~mhr3/libzeitgeist/fix-986230
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Mikkel Kamstrup Erlandsen		2012-04-27	Approve on 2012-04-27
Review via email: mp+103845@code.staging.launchpad.net

Description of the change

Async methods linger indefinitely if getting the dbus proxy right after construction returns an error.

Revision history for this message

Mikkel Kamstrup Erlandsen (kamstrup) wrote on 2012-04-27:

60 + else if (priv->log_error)
61 + {

Should there not be a g_clear_error() in this branch?

195 + goto process_pending_calls;

Why this? We supposedly don't have a proxy to process the pending calls on?

review: Needs Information

Revision history for this message

Michal Hruby (mhr3) wrote on 2012-04-27:

> 60 + else if (priv->log_error)
> 61 + {
>
> Should there not be a g_clear_error() in this branch?
>

Nope, we still keep the error for later calls.

> 195 + goto process_pending_calls;
>
> Why this? We supposedly don't have a proxy to process the pending calls on?

We don't have proxy, but we do have an error we'll throw.

Revision history for this message

Siegfried Gevatter (rainct) wrote on 2012-04-27:

2012/4/27 Mikkel Kamstrup Erlandsen <email address hidden>:
> 60 + else if (priv->log_error)
> 61 + {
>
> Should there not be a g_clear_error() in this branch?

No, the error is still needed.

> 195 + goto process_pending_calls;
>
> Why this? We supposedly don't have a proxy to process the pending calls on?

The code in dispatch_method will run the callback itself giving it the
error code we received when connecting.

Revision history for this message

Mikkel Kamstrup Erlandsen (kamstrup) wrote on 2012-04-27:

I grilled mhr3 a bit on IRC about this, and I am happy now. This should also be safe for SRU because it only changes the error paths (to the better!).

review: Approve