Review Request 122168: Also replace '.' by '_' when registering dbus path names from X_KDE_Library

[Hugo Pereira Da Costa]

> On Jan. 20, 2015, 4:16 p.m., David Faure wrote:
> > src/kcmoduleproxy.cpp, line 99
> > <https://git.reviewboard.kde.org/r/122168/diff/1/?file=343470#file343470line99>
> >
> >     ... while this line is happy with dots.
> 
> Hugo Pereira Da Costa wrote:
>     ok. Got it now. Thanks !
> 
> Hugo Pereira Da Costa wrote:
>     Stupid question: does it not also apply to the two lines above (replacement of '-' and '/' by '_') ? And if yes, should I also 'fix' that in the same review ?
> 
> Hugo Pereira Da Costa wrote:
>     ok. Ditch this. While trying not to do the replacements for dbusservice (code below), it seems that '.' are also not allowed here. The crash I get is: 
>     
>     process 7571: arguments to dbus_message_new_method_call() were incorrect, assertion "destination == NULL || _dbus_check_is_valid_bus_name (destination)" failed in file dbus-message.c line 1266.
>     
>     For the code: 
>             QString name = modInfo.handle();
>             name.replace(QLatin1Char('-'), QLatin1Char('_')); // hyphen is not allowed in dbus, only [A-Z][a-z][0-9]_
>             name.replace(QLatin1Char('/'), QLatin1Char('_')); // same goes for '/'
>             dbusService = QLatin1String(".kde.internal.KSettingsWidget_") + name;
>     
>             // for the path, we also need to replace '.' characters, which are not allowed in dbus
>             name.replace(QLatin1Char('.'), QLatin1Char('_'));
>             dbusPath = QLatin1String("/internal/KSettingsWidget/") + name;
> 
> David Faure wrote:
>     so what's the actual dbusService string that you're trying to register?
>     
>     http://dbus.freedesktop.org/doc/dbus-specification.html section "Bus names" has the list of rules.
> 
> Hugo Pereira Da Costa wrote:
>     The library path (in the .desktop file) is "X-KDE-Library=org.kde.kdecoration2/breezedecoration"
>     (the ork.kde.kdecoration2 subpath is a requirement from kwin)
>     
>     Right now (without patch) the current code converts this into 
>     
>     dbusPath = /internal/KSettingsWidget/org.kde.kdecoration2_breezedecoration <- crash
>     dbusservice = org.kde.internal.KSettingsWidget_org.kde.kdecoration2_breezedecoration
>     which also crashes

ok. Sorry. it seems I cannot reproduce the 'second' crash above, after a system reboot. 
I'll submit an updated patch in a sec. (reading the doc, it is true that the said dbusservice, including dots, should be valid.


- Hugo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122168/#review74403
-----------------------------------------------------------


On Jan. 20, 2015, 11:02 a.m., Hugo Pereira Da Costa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122168/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2015, 11:02 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcmutils
> 
> 
> Description
> -------
> 
> Also replace '.' by '_' when registering dbus path names from X_KDE_Library
> Rationale:
> kwin requires that configuration components for kdecoration gets installed as kcmodule in $KF5/lib/plugins/org.kde.kdecoration2
> 
> Trying to make the said module for breeze directly loadable from kcmshell5, I hit 
> 
> ASSERT failure in QDBusConnection::registerObject: "Invalid object path given"
> 
> 
> Diffs
> -----
> 
>   src/kcmoduleproxy.cpp 630217a 
> 
> Diff: https://git.reviewboard.kde.org/r/122168/diff/
> 
> 
> Testing
> -------
> 
> yes, with code not pushed yet in breeze decoration config.
> Before: crash (due to the above)
> After: no crash.
> 
> 
> Thanks,
> 
> Hugo Pereira Da Costa
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150121/acdb62bb/attachment-0001.html>