| Summary: | test-dbus-daemon process_id fails due to wrong message format on unset PID | ||
|---|---|---|---|
| Product: | dbus | Reporter: | LRN <lrn1986> |
| Component: | core | Assignee: | Simon McVittie <smcv> |
| Status: | RESOLVED FIXED | QA Contact: | D-Bus Maintainers <dbus> |
| Severity: | normal | ||
| Priority: | medium | CC: | lrn1986 |
| Version: | git master | Keywords: | patch |
| Hardware: | Other | ||
| OS: | Windows (All) | ||
| Whiteboard: | review? | ||
| i915 platform: | i915 features: | ||
| Attachments: |
Cripple W32 socket credentials reader to NOT set the process ID
Use dbus_set_error_from_message() to check for an error Use dbus_set_error_from_message() to check for an error update-activation-environment: produce better diagnostics on error various tests: produce better diagnostics on error |
||
|
Description
LRN
2016-06-23 12:18:27 UTC
Created attachment 124681 [details] [review] Use dbus_set_error_from_message() to check for an error This patch did fix the test for me. Comment on attachment 124681 [details] [review] Use dbus_set_error_from_message() to check for an error Review of attachment 124681 [details] [review]: ----------------------------------------------------------------- Looks basically right. Would you mind having a look for anywhere else that has the same bug? ::: test/dbus-daemon.c @@ +556,4 @@ > } > else > { > + g_assert_not_reached (); I'd prefer something like g_error ("Unexpected error: %s: %s", error.name, error.message); so we get better diagnostics. (In reply to Simon McVittie from comment #2) > Comment on attachment 124681 [details] [review] [review] > Use dbus_set_error_from_message() to check for an error > > Review of attachment 124681 [details] [review] [review]: > ----------------------------------------------------------------- > > Looks basically right. Would you mind having a look for anywhere else that > has the same bug? I would mind. Created attachment 124682 [details] [review] Use dbus_set_error_from_message() to check for an error v2: * Use g_error() for better diagnostic message on unexpected error Comment on attachment 124682 [details] [review] Use dbus_set_error_from_message() to check for an error Review of attachment 124682 [details] [review]: ----------------------------------------------------------------- Looks good, I'll merge it soon. (In reply to LRN from comment #3) > I would mind. No problem, I'll try to get to this myself eventually. Fixed in git for 1.10.10, 1.11.4. Thanks! Leaving this bug open (but marking it as not containing a relevant patch any more) since I still need to check whether we've made the same mistake anywhere else. Created attachment 125008 [details] [review] update-activation-environment: produce better diagnostics on error If dbus-daemon or systemd replied to our method call with an error, we would report it as "invalid arguments" instead of the true error name and message. Same root cause as <https://bugs.freedesktop.org/show_bug.cgi?id=96653>. --- Targeting 1.10 for this one since it's a user-facing diagnostic, unless reviewers feel that this change should only go to master. Created attachment 125009 [details] [review] various tests: produce better diagnostics on error Same root cause as <https://bugs.freedesktop.org/show_bug.cgi?id=96653>: we didn't check whether the message was in fact an error reply. --- Intended for 1.11 only, since this is only a test issue and doesn't cause any known test failures. (In reply to Simon McVittie from comment #8) > update-activation-environment: produce better diagnostics on error ... > Targeting 1.10 for this one since it's a user-facing diagnostic, unless > reviewers feel that this change should only go to master. It's been a month, and the change is fairly trivial, so I applied it unreviewed. (In reply to Simon McVittie from comment #9) > various tests: produce better diagnostics on error Again, it's been a month, so I applied this unreviewed. Fixed in git for 1.10.10, 1.11.4. |
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.