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)
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.
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.
(In reply to comment #2)
> I think you shouldn't use that method if you can still continue without dbus
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.
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.
We now have tp_dbus_daemon_dup(), which doesn't call exit(), so here's a branch that marks tp_get_bus() as deprecated:
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).
tp_get_bus() has been marked deprecated (again) for a while. There is exactly one call to it in the test suite.
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 ***