Bug 50418 - configuration: add <listen_if_possible> option
Summary: configuration: add <listen_if_possible> option
Status: RESOLVED MOVED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.5
Hardware: All All
: low enhancement
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL: http://cgit.collabora.com/git/user/al...
Whiteboard: review-, minor fixes needed
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-05-28 02:11 UTC by Alban Crequy
Modified: 2018-10-12 21:14 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Alban Crequy 2012-05-28 02:11:44 UTC
Introduce a <listen_if_possible> so the dbus-daemon won't just crash out if run on a kernel without support for the socket type we would like.

I will add an implementation soon.

Example of D-Bus configuration file:
  <listen>tcp:port=1234</listen>
  <listen_if_possible>tcp:port=1235</listen_if_possible>
  <listen_if_possible>pigeon:port=carrier,speed=110mph</listen_if_possible>

If the daemon cannot bind on port 1234, it will fail. But if it cannot bind on port 1235, or if it does not understand the last address, it will just ignore the error.
Comment 1 Alban Crequy 2012-05-28 02:27:07 UTC
Patch in URL. "make check" passes.
Comment 2 Simon McVittie 2012-06-05 04:32:33 UTC
Deferred to dbus-1.7. It's mostly useful for transports requiring new kernel features, and we don't yet have any of those.

It might be interesting to change the semantics of <listen_if_possible> to: if there are no <listen> directives at all, and every <listen_if_possible> directive failed, then that's still an error.

Then, we could do something like this in the default session.conf:

    <listen_if_possible>unix:tmpdir=/tmp</listen_if_possible>
    <listen_if_possible>windows-autolaunch:</listen_if_possible>

and it'd "do the right thing" on either Unix or Windows platforms?
Comment 3 Alban Crequy 2012-06-08 05:51:55 UTC
(In reply to comment #2)

> It might be interesting to change the semantics of <listen_if_possible> to: if
> there are no <listen> directives at all, and every <listen_if_possible>
> directive failed, then that's still an error.

I updated the branch with this semantic.
Comment 4 Simon McVittie 2012-06-15 07:33:15 UTC
This is looking good for 1.7.0, with some minor adjustments.

> +bus_config_parser_get_addresses_if_possible (BusConfigParser *parser)

This sounds like it has "do foo if possible" semantics, like g_try_malloc. I'd prefer "get_if_possible_addresses" or something?

> + "All <listen_if_possible> elements failed to listen"
> + " and there is no <listen> elements\n");

Grammar: "there are no".

This should be documented in the man page, too:

> +Same as <listen> but ignore any errors if the bus cannot listen on the
> +specified address.

I'd phrase it as:

Same as <listen>, but if the bus cannot listen on the specified address, it is skipped. After trying all configured addresses, it will report an error and exit if they were all unsuccessful.
Comment 5 Alban Crequy 2012-06-15 09:19:40 UTC
(In reply to comment #4)
> This is looking good for 1.7.0, with some minor adjustments.

Thanks for the review! I did the adjustments and pushed & rebase them in the branch listenifpossible2.
Comment 6 Simon McVittie 2013-05-10 16:30:55 UTC
(Sorry for the whitespace damage in quoted code. Blame cgit, or attach git-format-patch output.)

+ server = dbus_server_listen (link->data, NULL);
+ if (server == NULL)
+ {
+ link = _dbus_list_get_next_link (addresses_if_possible, link);
+ continue;
+ }
+
+ if (!setup_server (context, server, auth_mechanisms, NULL))
+ {
/* 1 */
+ link = _dbus_list_get_next_link (addresses_if_possible, link);
+ continue;
+ }
+
+ if (!_dbus_list_append (&context->servers, server))
/* 2 */
+ goto oom;
+
+ link = _dbus_list_get_next_link (addresses_if_possible, link);

At /* 1 */ and /* 2 */, the DBusServer is leaked. Both of these are also a bug in the existing code :-(

At /* 1 */ I think we should probably also be passing error as last parameter and doing "goto failed" on error, just like the <listen> code path. Justification: in your example in Comment #0, we want to carry on regardless if port 1235 is unavailable, or if the carrier pigeon transport is unavailable. If port 1235 is available and ought to work, but we don't have enough memory to set up a server on it properly, I don't think that's really the same thing, though: we should just give up.

Possibly "out of memory" from dbus_server_listen() should even be special-cased too? I'm not sure that that would be worth the effort, though.

Other than that, this looks good. Sorry it got stalled for so long.
Comment 7 Simon McVittie 2013-05-10 16:31:32 UTC
> Both of these are also a bug in the existing code

... not that it really matters in the existing code, because the dbus-daemon is about to exit unsuccessfully anyway.
Comment 8 GitLab Migration User 2018-10-12 21:14:02 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/69.


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.