Summary: | init_session_address() should raise a DBusError instead of doing _dbus_warn() | ||
---|---|---|---|
Product: | dbus | Reporter: | Guy Harris <guy> |
Component: | core | Assignee: | D-Bus Maintainers <dbus> |
Status: | RESOLVED MOVED | QA Contact: | D-Bus Maintainers <dbus> |
Severity: | normal | ||
Priority: | medium | CC: | chengwei.yang.cn, msniko14 |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
Whiteboard: | review-, Mac testing also needed | ||
i915 platform: | i915 features: | ||
Attachments: | [PATCH] Mac: do not warn and ignore error for init session bus address |
Description
Guy Harris
2014-01-24 19:33:05 UTC
(In reply to comment #0) > calls to try to open the "dbus-system" device calling dbus_bus_get with > DBUS_BUS_SYSTEM as the first argument; > > the system perhaps not being configured properly; This is probably not appropriate to do by default on Mac OS, since the operating system as shipped by its vendor (i.e. Apple) doesn't have a system bus. Eavesdropping on the D-Bus system bus doesn't work without intrusive and insecure reconfiguration, so I would suggest having "list my devices" API calls in pcap/wireshark not try that. Having said that, if you used dbus_bus_get(DBUS_BUS_SESSION, .) the same code would run, as far as I can see. It seems somewhat inappropriate that libdbus' Mac OS launchd support is behaving like this: given that it can fail, I'd somewhat prefer it if it used a pseudo-protocol (maybe "launchd:"?) in the same way as X11's "autolaunch:", which runs at a stage in the connection process that *can* handle errors. > assuming that only serious "I can't function" errors will show up on the > standard error This is not a safe assumption for generic libraries. stderr is the only universal fallback for handling serious errors, unfortunately. However, libdbus is not meant to log to stderr unless: * a caller is using it wrong ("programmer error" that cannot be corrected at runtime, only by changing the caller's source code); or * it has detected internal breakage (assertion failure) > init_session_address() calling _dbus_warn() if > _dbus_lookup_session_address() sets "supported" to TRUE and returns FALSE, > causing exactly such an error to go to the standard error. It looks as though the bug here is that init_session_address() and init_connections_unlocked() have no better way to report an error. They should use DBusError for structured error-reporting instead, like internal_bus_get() does. Patches welcome... (I'd write one myself, but then I wouldn't be able to review it, and it'd sit in Bugzilla forever.) Created attachment 93825 [details] [review] [PATCH] Mac: do not warn and ignore error for init session bus address This is not a major bug but does cause some inconveniences for lib users, it's better we can return an error rather than print to stderr and ignore the error. I didn't verify this patch but pass "make check" since I have no Mac machine. Comment on attachment 93825 [details] [review] [PATCH] Mac: do not warn and ignore error for init session bus address Review of attachment 93825 [details] [review]: ----------------------------------------------------------------- > I didn't verify this patch but pass "make check" since I have no Mac machine. Regardless of any reviews, I won't merge this patch until it has been tested in the situation it is intended to fix. ::: dbus/dbus-bus.c @@ -159,4 @@ > /* So it's not in the environment - let's try a platform-specific method. > * On MacOS, this involves asking launchd. On Windows (not specified yet) > * we might do a COM lookup. > - * Ignore errors - if we failed, fall back to autolaunch. */ Is there any platform where supported will be set to TRUE, an error might occur, but autolaunch might later turn out to work? If so, you've just broken it. @@ +434,3 @@ > { > + if (!dbus_error_is_set (error)) > + _DBUS_SET_OOM (error); If init_connections_unlocked() takes a DBusError parameter, then it should always be responsible for filling it - in particular, it should call _DBUS_SET_OOM() where appropriate. DBusError is meant to be like GLib's GError, see the GLib documentation for policies and conventions. Think of returning FALSE with the error set as being like throwing an exception in higher-level languages. undoing abusive changes -- 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/94. |
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.