Re: Review Request: KOpenWithDialog: Quote paths selected in the file dialog
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103602/#review9818 --- Ship it! Yep we want to quote only paths from the file dialog, not command lines types by the user or selected in the application tree. So this looks good. - David Faure On Jan. 1, 2012, 9:49 p.m., Ingomar Wesp wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103602/ --- (Updated Jan. 1, 2012, 9:49 p.m.) Review request for kdelibs. Description --- KOpenWithDialog expects the input in its line edit to be shell-quoted. Currently, however, when the user picks a path via the provided file dialog, whitespace and other special characters in the path are inserted verbatim. This leads to an error when the user tries to select an executable whose path contains such characters. This tiny patch addresses the issue by adding a private slot that runs all paths coming from the file dialog through KShell::quoteArg(). I believe this solution is better than the one I originally proposed in the linked bug report, since it avoids the ambiguity of having to guess whether whitespace belongs to the path or an argument to the executable. This addresses bug 281952. http://bugs.kde.org/show_bug.cgi?id=281952 Diffs - kio/kfile/kopenwithdialog.h 86c02ab kio/kfile/kopenwithdialog.cpp b5ebbbf Diff: http://git.reviewboard.kde.org/r/103602/diff/diff Testing --- Thanks, Ingomar Wesp
Re: Review Request: #include fixx11h.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103638/ --- (Updated Jan. 14, 2012, 6:16 p.m.) Review request for KDE Base Apps and Hugo Pereira Da Costa. Description --- Add #include fixx11h.h in some places where it was missing. This improves buildability. Diffs - kcminit/main.cpp 68bfb0530514d27d4c7b5cbb0b4bd509c1b4a8da kcontrol/access/kaccess.h c364b25103fa8ac6ce5f8a94a926a63fac2a68e4 kcontrol/access/kcmaccess.cpp 64a68039a39f4216f979c3658f5c1e3421ce4e8b kcontrol/bell/bell.cpp 3d9395f62702c45feb8bcb6e071e35d7deb59f49 kcontrol/fonts/fonts.cpp 61886d4181b8c442f81d540f591bd7149fefb735 kcontrol/input/kapplymousetheme.cpp c9bc511b7e60d95f29fceaab6a89ef9e7c1a90e8 kcontrol/input/main.cpp 0f9f33a6035bcd807130793901897f8d010a038f kcontrol/input/mouse.cpp cebb1747288e7173855d47194508b27eddf60b95 kcontrol/input/xcursor/legacytheme.cpp 28d7f2a02471b8d983480f243e93ebe71b2971b4 kcontrol/input/xcursor/previewwidget.cpp aff149bc7ed34a527c56a05feac57fab9761a561 kcontrol/input/xcursor/thememodel.cpp d69dde06684b07428f531422ef06a12e181a566c kcontrol/input/xcursor/themepage.cpp 0f678ed1c99cc8c57fd32b5bdf86beca356befba kcontrol/input/xcursor/xcursortheme.cpp 2fc7504b93443b13962652945da856d4d94e8e4e kcontrol/keyboard/kcmmisc.cpp 382f026fbc7290cdcca1838671679b7e7721fd2b kcontrol/keyboard/keyboard_hardware.cpp 9f9c026db882c96655d67904caf0abe055d4b25b kcontrol/keyboard/xinput_helper.cpp 2971d20c3c049e9712e75f5145e39f0952f23779 kcontrol/kfontinst/lib/FcEngine.cpp 8b74fceb426b19f64af5323f4e7e016b8005f10e kcontrol/krdb/krdb.cpp 92d84e9a01863154f8546bdcb93b5e23acc0d5fa kcontrol/randr/krandrapp.cpp af536710ba4e51c1846cfd576788292192047947 kcontrol/randr/module/randrpolltest.cpp 30d689e8df39db02496a6d9641d04e32f521d5ba kcontrol/style/kcmstyle.cpp b8f46be3a42a837a13fdf6c502e136511366 kdm/kcm/background/bgrender.cpp 17cf33deb71b81fdbd3232a011915f8d6786c985 kdm/kfrontend/krootimage.cpp e4ddc858cbc907373159dd96e46c11fdde8a681e khotkeys/libkhotkeysprivate/actions/keyboard_input_action.cpp b3b1ec36713ebaa411dde742cc6503ff420b7134 khotkeys/libkhotkeysprivate/triggers/gestures.cpp e70c074f4eb7c08d58e2fde9fe3095cabb9ac27d khotkeys/libkhotkeysprivate/windows_handler.cpp eb02374affedf45309bdb122df0560b3a7d3e5af kinfocenter/Modules/base/os_base.h 9c903ea009f40b46492dc720978a60d4f6655097 kinfocenter/Modules/opengl/opengl.cpp 7df2b172fa539bd28f09b9f27d0c486b1cb9c2d7 klipper/klipper.cpp aafe6160d9a88448cd5e91390f0a695b07be61e7 krunner/krunnerdialog.cpp 007887fbafefb6ce0269fbba21ea50a204606446 krunner/lock/lockprocess.cc 65c7f1da4f847cf516232bcd392683042b5957a7 krunner/screensaver/xautolock.cpp 71242156c2b499a6a3ca8cd415ee6148b2be259c krunner/startupid.cpp a436183b27530142edfdc4c2aeecf6cba5417333 kscreensaver/libkscreensaver/kscreensaver.cpp 55bcbeae15343da8774beeb4513aaa44ee423b8f ksmserver/fadeeffect.h 8d45ebc05517dbb8887e7100dd0e82a809f6521b ksmserver/fadeeffect.cpp 8b948345ed8aebe70a282a49a849dac53b0d52b8 ksmserver/kcheckrunning.cpp f0648fc26ab289c4a2a0acc6ba8912cc20e30856 ksmserver/legacy.cpp 62a4672e261222a50747dde05f83887a8b5b7eed ksmserver/logouteffect.cpp a2fc06057881dd9ef78d4a01e2cea8bf1a9abfbf ksplash/ksplashqml/SplashApp.h 9b63c4e53003ffa6c55afce7c630c9e03ea1f872 ksplash/ksplashqml/main.cpp ed409e24ab8f15d8cbb6c8b66fed9a6d744af30a ksplash/simple/main.cpp 0e730bfbac7702bb24439672e8b69909a460d7fe kstyles/oxygen/oxygenshadowhelper.cpp 28cc651c99576b0144b66e9845b2e16a23aa03aa kstyles/oxygen/oxygenstylehelper.cpp 24710cd6d8bef5943b7a7dd493b7091ea279aaa3 ksystraycmd/main.cpp 9a72389b9535c4caf6158cc91413427badffb2a8 kwin/atoms.h 95e1bde9ee9be5489d85b1cfa3253e9cbdbadfcd kwin/clients/b2/b2client.cpp 6b529964239797188a06f7081ff73c9fe25f17bc kwin/clients/oxygen/oxygenclient.cpp cd94eb48bf6559a576a8820fbf7b04149512da5a kwin/clients/oxygen/oxygensizegrip.cpp 221ee74b79d9020bed5815c933e09145205c7ad7 kwin/effects/resize/resize.cpp 84bdd7f7dbc99457a0e6df4eefcdffdb31c9342f kwin/effects/showfps/showfps.cpp 5161401f7da4adb182b57fbfc5301d5a2f6b0028 kwin/effects/showpaint/showpaint.cpp f689b1c64e2599c3b9a5a8df11429012425b40ab kwin/kcmkwin/kwindecoration/preview.cpp a3d425674db23059eb03f40840ff540afa9d1d6f kwin/kcmkwin/kwinoptions/mouse.cpp 51a80f9d8d1334cfa4456ebf3f3e45d6b6224570 kwin/kcmkwin/kwinoptions/windows.cpp 30c94c0ae1b857f2b7228a3dba75d196835bbc65 kwin/killer/killer.cpp d37a654404ecdee458ad27f9961dc24f05665246 kwin/killwindow.cpp 57e15e525a4428b80f5bc3c28d2934d7a4caa401 kwin/libkwineffects/kwinglobals.cpp 575813ee3f987c54e76362b8c6598ee5d3952d1e kwin/opengltest/opengltest.cpp eda7b51ec1e0bd85564bc4c1d7d1ab1c2502de7a kwin/outline.cpp 6ef499ce263bb8ac18a51961c7e18edea049f03c
Re: Review Request: Avoid QDBusConnection Qt warning message for each KUniqueApplication
On Saturday, 14. January 2012 22:36:44 Thiago Macieira wrote:
On Saturday, 14 de January de 2012 18.38.02, Martin Koller wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103699/
---
Review request for kdelibs.
Description
---
All KUniqueApplications issue the warning
QDBusConnection: session D-Bus connection created before QCoreApplication.
Application may misbehave. when runngin with Qt-4.8.0 (qWarning in
QDBusDefaultConnection ctor)
The patch avoids this by temporarily creating a QCoreApplication instance
If this is done before the fork, it's a REALLY BAD IDEA.
it is done after the fork (if there is a fork and not used with --nofork).
If it's done after the fork, why is the connection created this early? I
might
have written this code, but I certainly don't remember it anymore.
According to the header doc KUniqueApplication shall be used like this:
...
*if (!KUniqueApplication::start()) {
* fprintf(stderr, myAppName is already running!\n);
* return 0;
*}
*KUniqueApplication a;
*return a.exec();
So one of the very first things the static start() method does is to
open the session dbus, and this is before a QCoreApplication is available.
So my idea here was to create one and delete it afterwards, which should
result in the same state as before the call to tryToInitDBusConnection()
If that is a bad idea, i'd like to learn why and what can be done otherwise.
(One other workaround to get rid of the warning message would be to set up
an own message handler to discard this message, but I'd rather not do that...)
--
Best regards/Schöne Grüße
Martin
A: Because it breaks the logical sequence of discussion
Q: Why is top posting bad?
() ascii ribbon campaign - against html e-mail
/\ www.asciiribbon.org - against proprietary attachments
Geschenkideen, Accessoires, Seifen, Kulinarisches: www.bibibest.at
signature.asc
Description: This is a digitally signed message part.
Re: Review Request: Avoid QDBusConnection Qt warning message for each KUniqueApplication
On Sunday, 15 de January de 2012 11.47.10, Martin Koller wrote:
*if (!KUniqueApplication::start()) {
* fprintf(stderr, myAppName is already running!\n);
* return 0;
*}
*KUniqueApplication a;
*return a.exec();
So one of the very first things the static start() method does is to
open the session dbus, and this is before a QCoreApplication is available.
It should open a session bus connection, but not *the*
QDBusConnection::sessionBus() connection.
I know I designed the code like that. You can't open it before because that
would mean the file descriptor is opened in the wrong process. So where is the
warninig coming from? Did I place the warning in the wrong object in QtDBus?
Or is there something *else* creating that connection?
Your description still requires the QCoreApplication object to be created
before the fork. It doesn't matter that you're going to close that open and
open a new one after the fork: it's still a bad idea. Some Unix operations do
not survive forks, like creation of threads. If you do that, I'm pretty sure
QProcess will be irreparably broken.
--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center
PGP/GPG: 0x6EF45358; fingerprint:
E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
signature.asc
Description: This is a digitally signed message part.
Re: Review Request 103602
On 01/-10/-28163 08:59 PM, Ingomar Wesp wrote: I don't mean to nag, but could someone please take another look at https://git.reviewboard.kde.org/r/103602/. No longer necessary. Thanks for reviewing, David! -- Ingo
Re: Review Request: Avoid QDBusConnection Qt warning message for each KUniqueApplication
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103699/ --- (Updated Jan. 15, 2012, 5:28 p.m.) Review request for kdelibs. Changes --- Don't create QCoreApplication but an own session bus QDBusConnection object. Description --- All KUniqueApplications issue the warning QDBusConnection: session D-Bus connection created before QCoreApplication. Application may misbehave. when runngin with Qt-4.8.0 (qWarning in QDBusDefaultConnection ctor) The patch avoids this by temporarily creating a QCoreApplication instance Diffs (updated) - kdeui/kernel/kuniqueapplication.cpp 777fc35 Diff: http://git.reviewboard.kde.org/r/103699/diff/diff Testing --- running kdepasswd Thanks, Martin Koller
Re: Review Request: Avoid QDBusConnection Qt warning message for each KUniqueApplication
On Sunday, 15. January 2012 13:18:58 Thiago Macieira wrote:
On Sunday, 15 de January de 2012 11.47.10, Martin Koller wrote:
*if (!KUniqueApplication::start()) {
* fprintf(stderr, myAppName is already running!\n);
* return 0;
*}
*KUniqueApplication a;
*return a.exec();
So one of the very first things the static start() method does is to
open the session dbus, and this is before a QCoreApplication is available.
It should open a session bus connection, but not *the*
QDBusConnection::sessionBus() connection.
I know I designed the code like that. You can't open it before because that
would mean the file descriptor is opened in the wrong process. So where is
the
warninig coming from?
qWarning is in QDBusDefaultConnection ctor
Did I place the warning in the wrong object in QtDBus?
Or is there something *else* creating that connection?
no, it's the call to QDBusConnection::sessionBus()
which creates the QDBusDefaultConnection object which throws that warning.
Your description still requires the QCoreApplication object to be created
before the fork. It doesn't matter that you're going to close that open and
open a new one after the fork: it's still a bad idea. Some Unix operations do
not survive forks, like creation of threads. If you do that, I'm pretty sure
QProcess will be irreparably broken.
Sorry, I don't understand that.
Nevertheless, I tried now what you've suggested above, namely to create my own
QDBusConnection object to the session bus. This seems to work now, however
as I'm new to DBus, I do not fully understand everything.
What I've found out is that the KUniqueApplication's DBus call to
/MainApplication
only works if I create a QDBusConnection object on the session bus only if it
uses the same name as the Qt-internal QDBusDefaultConnection object, which is
qt_default_session_bus, even when I would create my QDBusConnection objects
in the way that they live as long as the app lives.
Does that mean that a registered Object on DBus is linked to the
QDBusConnection's name ?
I've updated the diff https://git.reviewboard.kde.org/r/103699/diff/2/
--
Best regards/Schöne Grüße
Martin
A: Because it breaks the logical sequence of discussion
Q: Why is top posting bad?
() ascii ribbon campaign - against html e-mail
/\ www.asciiribbon.org - against proprietary attachments
Geschenkideen, Accessoires, Seifen, Kulinarisches: www.bibibest.at
signature.asc
Description: This is a digitally signed message part.
Re: Review Request: #include fixx11h.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103638/#review9843 --- As far as Oxygen is concerned, feel free to ship it. - Hugo Pereira Da Costa On Jan. 15, 2012, 12:53 p.m., Erik Sigra wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103638/ --- (Updated Jan. 15, 2012, 12:53 p.m.) Review request for KDE Base Apps and Hugo Pereira Da Costa. Description --- Add #include fixx11h.h in some places where it was missing. This improves buildability. Diffs - kcminit/main.cpp 68bfb05 kcontrol/access/kaccess.h c364b25 kcontrol/access/kcmaccess.cpp 64a6803 kcontrol/bell/bell.cpp 3d9395f kcontrol/fonts/fonts.cpp 61886d4 kcontrol/input/kapplymousetheme.cpp c9bc511 kcontrol/input/main.cpp 0f9f33a kcontrol/input/mouse.cpp cebb174 kcontrol/input/xcursor/legacytheme.cpp 28d7f2a kcontrol/input/xcursor/previewwidget.cpp aff149b kcontrol/input/xcursor/thememodel.cpp d69dde0 kcontrol/input/xcursor/themepage.cpp 0f678ed kcontrol/input/xcursor/xcursortheme.cpp 2fc7504 kcontrol/keyboard/kcmmisc.cpp 382f026 kcontrol/keyboard/keyboard_hardware.cpp 9f9c026 kcontrol/keyboard/xinput_helper.cpp 2971d20 kcontrol/kfontinst/lib/FcEngine.cpp 8b74fce kcontrol/krdb/krdb.cpp 92d84e9 kcontrol/randr/krandrapp.cpp af53671 kcontrol/randr/module/randrpolltest.cpp 30d689e kcontrol/style/kcmstyle.cpp b8f46be kdm/kcm/background/bgrender.cpp 17cf33d kdm/kfrontend/krootimage.cpp e4ddc85 khotkeys/libkhotkeysprivate/actions/keyboard_input_action.cpp b3b1ec3 khotkeys/libkhotkeysprivate/triggers/gestures.cpp e70c074 khotkeys/libkhotkeysprivate/windows_handler.cpp eb02374 kinfocenter/Modules/base/os_base.h 9c903ea kinfocenter/Modules/opengl/opengl.cpp 7df2b17 klipper/klipper.cpp aafe616 krunner/krunnerdialog.cpp 007887f krunner/lock/lockprocess.cc 65c7f1d krunner/screensaver/xautolock.cpp 7124215 krunner/startupid.cpp a436183 kscreensaver/libkscreensaver/kscreensaver.cpp 55bcbea ksmserver/fadeeffect.h 8d45ebc ksmserver/fadeeffect.cpp 8b94834 ksmserver/kcheckrunning.cpp f0648fc ksmserver/legacy.cpp 62a4672 ksmserver/logouteffect.cpp a2fc060 ksplash/ksplashqml/SplashApp.h 9b63c4e ksplash/ksplashqml/main.cpp d59bff8 ksplash/simple/main.cpp 0e730bf kstyles/oxygen/oxygenshadowhelper.cpp 28cc651 kstyles/oxygen/oxygenstylehelper.cpp 24710cd ksystraycmd/main.cpp 9a72389 kwin/atoms.h 95e1bde kwin/clients/b2/b2client.cpp 6b52996 kwin/clients/oxygen/oxygenclient.cpp cd94eb4 kwin/clients/oxygen/oxygensizegrip.cpp 221ee74 kwin/effects/resize/resize.cpp 84bdd7f kwin/effects/showfps/showfps.cpp 5161401 kwin/effects/showpaint/showpaint.cpp f689b1c kwin/kcmkwin/kwindecoration/preview.cpp a3d4256 kwin/kcmkwin/kwinoptions/mouse.cpp 51a80f9 kwin/kcmkwin/kwinoptions/windows.cpp 30c94c0 kwin/killer/killer.cpp d37a654 kwin/killwindow.cpp 57e15e5 kwin/libkwineffects/kwinglobals.cpp 575813e kwin/opengltest/opengltest.cpp eda7b51 kwin/outline.cpp 6ef499c kwin/overlaywindow.h 14d2d58 kwin/tabbox/tabboxhandler.cpp e91ea71 kwin/tools/decobenchmark/preview.cpp 429cbd2 kwin/tools/test_gravity.cpp 4ddb136 kwin/utils.cpp a9abc5d kwin/workspace.h 40562d4 libs/kephal/service/xrandr12/randrdisplay.h 3a6392c libs/kworkspace/kdisplaymanager.cpp 28fabfc plasma/desktop/shell/plasmaapp.cpp 7abd8fc plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.cpp 4257202 plasma/generic/applets/systemtray/protocols/fdo/x11embedcontainer.cpp 1826512 plasma/generic/dataengines/mouse/cursornotificationhandler.h 7b4d3eb plasma/netbook/shell/plasmaapp.cpp 22c54b2 powerdevil/daemon/actions/dpms/powerdevildpmsaction.cpp a16bf7e powerdevil/daemon/backends/upower/xrandrbrightness.h 875c667 Diff: http://git.reviewboard.kde.org/r/103638/diff/diff Testing --- Thanks, Erik Sigra
Re: Review Request: Avoid QDBusConnection Qt warning message for each KUniqueApplication
On Sunday, 15 de January de 2012 18.28.28, Martin Koller wrote: qWarning is in QDBusDefaultConnection ctor Did I place the warning in the wrong object in QtDBus? Or is there something *else* creating that connection? no, it's the call to QDBusConnection::sessionBus() which creates the QDBusDefaultConnection object which throws that warning. I know. My question is: why was QDBusConnection::sessionBus() called before QCoreApplication was created? Can't we fix *that* by initialising it later? Your description still requires the QCoreApplication object to be created before the fork. It doesn't matter that you're going to close that open and open a new one after the fork: it's still a bad idea. Some Unix operations do not survive forks, like creation of threads. If you do that, I'm pretty sure QProcess will be irreparably broken. Sorry, I don't understand that. Nevertheless, I tried now what you've suggested above, namely to create my own QDBusConnection object to the session bus. This seems to work now, however as I'm new to DBus, I do not fully understand everything. What I've found out is that the KUniqueApplication's DBus call to /MainApplication only works if I create a QDBusConnection object on the session bus only if it uses the same name as the Qt-internal QDBusDefaultConnection object, which is qt_default_session_bus, even when I would create my QDBusConnection objects in the way that they live as long as the app lives. Does that mean that a registered Object on DBus is linked to the QDBusConnection's name ? No, it means you really did not understand the code you're trying to change. Here's *should* go on: 1) KUniqueApplication connects to D-Bus using a private connection 2) it checks if the application is already running. If it is and we would fork, we simply ask the running instance to show a new main window. Then we exit. 3) if it wasn't running, it forks. The parent process waits for the child process to register itself on the bus using the same private connection. The child process closes its end of the private connection. 4) the child process initialises the QCoreApplication object 5) the child process opens the default, shared session bus connection and registers itself on the bus 6) the parent process notices the child is up and running, so it exits with _exit(0) (not exit(0)). You're getting the warning because steps 4 and 5 are inverted. Your patch should de-invert them by making the very first call to QDBusConnection::sessionBus() happen after the QCoreApplication's constructor has finished running. That means it can run in the KApplication constructor body or in the KUniqueApplication constructor body, but not before. I've updated the diff https://git.reviewboard.kde.org/r/103699/diff/2/ I can't find my password. Someone convinced me a while ago to use passwords generated with keepassx, especially because the KDE accounts kept getting locked because of passwords being too easy to crack. Can you send the patch? -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
