Bug 18832

Summary: [ABI break] Tp-glib shared lib calls exit()
Product: Telepathy Reporter: Brian Pepple <bpepple>
Component: tp-glibAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED DUPLICATE QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium    
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard: ABI-break
i915 platform: i915 features:
Bug Depends on: 24114    
Bug Blocks:    
Attachments: Patch to remove exit and return NULL in tp_get_bus in case of error.\

Description Brian Pepple 2008-12-01 09:17:26 UTC
Noticed when I ran rpmlint against the latest rpm that tp-glib is calling exit() even though it's a shared lib, and doing so from a library is strongly discouraged since it prevents the calling program from handling the error, reporting it to the user, closing files properly, and cleaning up any state that the program has. It is preferred for the library to return an actual error code and let the calling program decide how to handle the situation.

Looks like it called in telepathy-glib/dbus.c:
DBusGConnection *tp_get_bus (void)
Comment 1 Sunil Mohan Adapa 2008-12-25 23:35:17 UTC
Created attachment 21487 [details] [review]
Patch to remove exit and return NULL in tp_get_bus in case of error.\

Spicebird is using tp-glib and does more things than just IM. So, when dbus connection fails, we would like handle the error and continue without IM. tp-glib however quits on dbus connection failure.

For my purpose, this simple patch that removes the exit statement and returns NULL in tp_get_bus will suffice.
Comment 2 Xavier Claessens 2008-12-26 01:42:16 UTC
I think you shouldn't use that method if you can still continue without dbus connection. Even with your patch you'll get a warning instead of a GError you can report to the user.

IMO in most telepathy programs failing at getting the dbus connection is fatal because we can't do anything else (use g_assert instead of exit?). But I agree that we could add another function that is non-fatal and return a GError instead. Probably libempathy should use it too. For example, trying to send a file to a contact from nautilus without dbus running shouldn't terminate nautilus, but an error could be displayed to the user.
Comment 3 Sunil Mohan Adapa 2008-12-26 02:25:27 UTC
(In reply to comment #2)
> I think you shouldn't use that method if you can still continue without dbus
> connection. 

I didn't consider this because if in future, tp_get_bus changes is behavior, the application might break. Perhaps I should directly use dbus_g_get_bus.

> Even with your patch you'll get a warning instead of a GError you
> can report to the user.

Yes, not returning the GError is not right. 

> 
> IMO in most telepathy programs failing at getting the dbus connection is fatal
> because we can't do anything else (use g_assert instead of exit?). But I agree
> that we could add another function that is non-fatal and return a GError
> instead. Probably libempathy should use it too. For example, trying to send a
> file to a contact from nautilus without dbus running shouldn't terminate
> nautilus, but an error could be displayed to the user.
> 
Comment 4 Simon McVittie 2009-01-12 04:51:43 UTC
Using dbus_g_get_bus in Spicebird would be appropriate, yes. We should probably deprecate tp_get_bus in favour of something with error handling.
Comment 5 Dafydd Harries 2009-09-22 09:22:31 UTC
We now have tp_dbus_daemon_dup(), which doesn't call exit(), so here's a branch that marks tp_get_bus() as deprecated:

http://git.collabora.co.uk/?p=user/daf/telepathy-glib;a=shortlog;h=refs/heads/deprecate-tp_get_bus

Comment 6 Simon McVittie 2009-09-28 10:46:51 UTC
You typo'd deprecated, and deprecated functions should have an #ifndef TP_DISABLE_DEPRECATED guard and G_GNUC_DEPRECATED.

However, this means we need to eradicate use of tp_get_bus() from telepathy-glib's own examples and tests, and the rest of the Telepathy stack, first (and tbh, deprecating it while still using it in our own CMs would be hypocritical).

I'm working on a branch to remove most instances of it from telepathy-glib (IMO we should keep exactly one use of it in the tests, and compile the tests with -Wno-deprecated).
Comment 7 Will Thompson 2011-10-24 07:18:43 UTC
tp_get_bus() has been marked deprecated (again) for a while. There is exactly one call to it in the test suite.
Comment 8 Simon McVittie 2012-02-01 10:49:23 UTC
This will finally be fixed on the 'next' branch as part of the first round of ABI breaks for Telepathy 1.0.

*** This bug has been marked as a duplicate of bug 31668 ***

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.