Re: Review Request 122393: Fix dangling pointer and possible memory leak in knotifications
On Feb. 2, 2015, 9:05 p.m., Martin Klapetek wrote:
src/notifybypopup.cpp, lines 485-495
https://git.reviewboard.kde.org/r/122393/diff/2/?file=346479#file346479line485
Can you explain this change in a bit more detail?
Well.. I just randomly notice this. line 470 might return and watcher won't be
deleted in that case.
- Xuetian
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/#review75238
---
On Feb. 2, 2015, 8:59 p.m., Xuetian Weng wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/
---
(Updated Feb. 2, 2015, 8:59 p.m.)
Review request for KDE Frameworks and Martin Klapetek.
Bugs: 342752
https://bugs.kde.org/show_bug.cgi?id=342752
Repository: knotifications
Description
---
- Follow code would crash with current code, because notificationQueue may
contain dangling pointer to KNotification.
#include QApplication
#include KNotification
int main(int argc, char* argv[]) {
QApplication app(argc,argv);
do {
KNotification a(NewMessage);
a.setComponentName(kwrited); // just a random component for test
a.sendEvent();
} while(0);
app.exec();
return 0;
}
This is fixed by remove item from queue in ::close.
- 342752 might happen because galagoNotifications may contain dangling
pointer.
This is fixed by using QPointer in galagoNotifications (instead of in
::close, because it will initiate a dbus call and others may be still willing
to wait for finished signal).
If KNotification is already deleted, don't call other related functions on
this KNotification.
- watcher is safer to call deleteLater on watcher first.
Diffs
-
src/notifybypopup.cpp 95937fc
Diff: https://git.reviewboard.kde.org/r/122393/diff/
Testing
---
Above code doesn't crash anymore.
Thanks,
Xuetian Weng
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122393: Fix dangling pointer and possible memory leak in knotifications
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/
---
(Updated Feb. 2, 2015, 9:35 p.m.)
Review request for KDE Frameworks and Martin Klapetek.
Bugs: 342752
https://bugs.kde.org/show_bug.cgi?id=342752
Repository: knotifications
Description
---
- Follow code would crash with current code, because notificationQueue may
contain dangling pointer to KNotification.
#include QApplication
#include KNotification
int main(int argc, char* argv[]) {
QApplication app(argc,argv);
do {
KNotification a(NewMessage);
a.setComponentName(kwrited); // just a random component for test
a.sendEvent();
} while(0);
app.exec();
return 0;
}
This is fixed by remove item from queue in ::close.
- 342752 might happen because galagoNotifications may contain dangling pointer.
This is fixed by using QPointer in galagoNotifications (instead of in ::close,
because it will initiate a dbus call and others may be still willing to wait
for finished signal).
If KNotification is already deleted, don't call other related functions on this
KNotification.
- watcher is safer to call deleteLater on watcher first.
Diffs (updated)
-
src/notifybypopup.cpp 95937fc
Diff: https://git.reviewboard.kde.org/r/122393/diff/
Testing
---
Above code doesn't crash anymore.
Thanks,
Xuetian Weng
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122393: Fix dangling pointer and possible memory leak in knotifications
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/
---
(Updated Feb. 2, 2015, 8:59 p.m.)
Review request for KDE Frameworks and Martin Klapetek.
Bugs: 342752
https://bugs.kde.org/show_bug.cgi?id=342752
Repository: knotifications
Description
---
- Follow code would crash with current code, because notificationQueue may
contain dangling pointer to KNotification.
#include QApplication
#include KNotification
int main(int argc, char* argv[]) {
QApplication app(argc,argv);
do {
KNotification a(NewMessage);
a.setComponentName(kwrited); // just a random component for test
a.sendEvent();
} while(0);
app.exec();
return 0;
}
This is fixed by remove item from queue in ::close.
- 342752 might happen because galagoNotifications may contain dangling pointer.
This is fixed by using QPointer in galagoNotifications (instead of in ::close,
because it will initiate a dbus call and others may be still willing to wait
for finished signal).
If KNotification is already deleted, don't call other related functions on this
KNotification.
- watcher is safer to call deleteLater on watcher first.
Diffs (updated)
-
src/notifybypopup.cpp 95937fc
Diff: https://git.reviewboard.kde.org/r/122393/diff/
Testing
---
Above code doesn't crash anymore.
Thanks,
Xuetian Weng
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122393: Fix dangling pointer and possible memory leak in knotifications
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/#review75238
---
Thanks for looking into this!
src/notifybypopup.cpp
https://git.reviewboard.kde.org/r/122393/#comment52047
```auto item``` -- ```auto item``` (as per the frameworks coding style)
src/notifybypopup.cpp
https://git.reviewboard.kde.org/r/122393/#comment52048
```KNotification* n``` -- ```KNotification *n``` (as per the frameworks
coding style)
src/notifybypopup.cpp
https://git.reviewboard.kde.org/r/122393/#comment52049
Can you explain this change in a bit more detail?
- Martin Klapetek
On Feb. 2, 2015, 9:59 p.m., Xuetian Weng wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/
---
(Updated Feb. 2, 2015, 9:59 p.m.)
Review request for KDE Frameworks and Martin Klapetek.
Bugs: 342752
https://bugs.kde.org/show_bug.cgi?id=342752
Repository: knotifications
Description
---
- Follow code would crash with current code, because notificationQueue may
contain dangling pointer to KNotification.
#include QApplication
#include KNotification
int main(int argc, char* argv[]) {
QApplication app(argc,argv);
do {
KNotification a(NewMessage);
a.setComponentName(kwrited); // just a random component for test
a.sendEvent();
} while(0);
app.exec();
return 0;
}
This is fixed by remove item from queue in ::close.
- 342752 might happen because galagoNotifications may contain dangling
pointer.
This is fixed by using QPointer in galagoNotifications (instead of in
::close, because it will initiate a dbus call and others may be still willing
to wait for finished signal).
If KNotification is already deleted, don't call other related functions on
this KNotification.
- watcher is safer to call deleteLater on watcher first.
Diffs
-
src/notifybypopup.cpp 95937fc
Diff: https://git.reviewboard.kde.org/r/122393/diff/
Testing
---
Above code doesn't crash anymore.
Thanks,
Xuetian Weng
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122393: Fix dangling pointer and possible memory leak in knotifications
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/
---
(Updated Feb. 2, 2015, 10:29 p.m.)
Status
--
This change has been marked as submitted.
Review request for KDE Frameworks and Martin Klapetek.
Bugs: 342752
https://bugs.kde.org/show_bug.cgi?id=342752
Repository: knotifications
Description
---
- Follow code would crash with current code, because notificationQueue may
contain dangling pointer to KNotification.
#include QApplication
#include KNotification
int main(int argc, char* argv[]) {
QApplication app(argc,argv);
do {
KNotification a(NewMessage);
a.setComponentName(kwrited); // just a random component for test
a.sendEvent();
} while(0);
app.exec();
return 0;
}
This is fixed by remove item from queue in ::close.
- 342752 might happen because galagoNotifications may contain dangling pointer.
This is fixed by using QPointer in galagoNotifications (instead of in ::close,
because it will initiate a dbus call and others may be still willing to wait
for finished signal).
If KNotification is already deleted, don't call other related functions on this
KNotification.
- watcher is safer to call deleteLater on watcher first.
Diffs
-
src/notifybypopup.cpp 95937fc
Diff: https://git.reviewboard.kde.org/r/122393/diff/
Testing
---
Above code doesn't crash anymore.
Thanks,
Xuetian Weng
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122393: Fix dangling pointer and possible memory leak in knotifications
On Feb. 2, 2015, 10:05 p.m., Martin Klapetek wrote:
src/notifybypopup.cpp, lines 485-495
https://git.reviewboard.kde.org/r/122393/diff/2/?file=346479#file346479line485
Can you explain this change in a bit more detail?
Xuetian Weng wrote:
Well.. I just randomly notice this. line 470 might return and watcher
won't be deleted in that case.
Ah yeah, good spot. Please add a comment there.
- Martin
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/#review75238
---
On Feb. 2, 2015, 9:59 p.m., Xuetian Weng wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/
---
(Updated Feb. 2, 2015, 9:59 p.m.)
Review request for KDE Frameworks and Martin Klapetek.
Bugs: 342752
https://bugs.kde.org/show_bug.cgi?id=342752
Repository: knotifications
Description
---
- Follow code would crash with current code, because notificationQueue may
contain dangling pointer to KNotification.
#include QApplication
#include KNotification
int main(int argc, char* argv[]) {
QApplication app(argc,argv);
do {
KNotification a(NewMessage);
a.setComponentName(kwrited); // just a random component for test
a.sendEvent();
} while(0);
app.exec();
return 0;
}
This is fixed by remove item from queue in ::close.
- 342752 might happen because galagoNotifications may contain dangling
pointer.
This is fixed by using QPointer in galagoNotifications (instead of in
::close, because it will initiate a dbus call and others may be still willing
to wait for finished signal).
If KNotification is already deleted, don't call other related functions on
this KNotification.
- watcher is safer to call deleteLater on watcher first.
Diffs
-
src/notifybypopup.cpp 95937fc
Diff: https://git.reviewboard.kde.org/r/122393/diff/
Testing
---
Above code doesn't crash anymore.
Thanks,
Xuetian Weng
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122393: Fix dangling pointer and possible memory leak in knotifications
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/
---
Review request for KDE Frameworks.
Bugs: 342752
https://bugs.kde.org/show_bug.cgi?id=342752
Repository: knotifications
Description
---
1. Follow code would crash with current code, because notificationQueue may
contain dangling pointer to KNotification.
#include QApplication
#include KNotification
int main(int argc, char* argv[]) {
QApplication app(argc,argv);
do {
KNotification a(NewMessage);
a.setComponentName(kwrited); // just a random component for test
a.sendEvent();
} while(0);
app.exec();
return 0;
}
This is fixed by remove item from queue in ::close.
2. 342752 might happen because galagoNotifications may contain dangling pointer.
This is fixed by using QPointer in galagoNotifications (instead of in ::close,
because it will initiate a dbus call and others may be still willing to wait
for finished signal).
If KNotification is already deleted, don't call other related functions on this
KNotification.
3. watcher is safer to call deleteLater on watcher first.
Diffs
-
src/notifybypopup.cpp 95937fc
Diff: https://git.reviewboard.kde.org/r/122393/diff/
Testing
---
Above code doesn't crash anymore.
Thanks,
Xuetian Weng
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122393: Fix dangling pointer and possible memory leak in knotifications
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/
---
(Updated Feb. 2, 2015, 8:57 p.m.)
Review request for KDE Frameworks.
Bugs: 342752
https://bugs.kde.org/show_bug.cgi?id=342752
Repository: knotifications
Description (updated)
---
- Follow code would crash with current code, because notificationQueue may
contain dangling pointer to KNotification.
#include QApplication
#include KNotification
int main(int argc, char* argv[]) {
QApplication app(argc,argv);
do {
KNotification a(NewMessage);
a.setComponentName(kwrited); // just a random component for test
a.sendEvent();
} while(0);
app.exec();
return 0;
}
This is fixed by remove item from queue in ::close.
- 342752 might happen because galagoNotifications may contain dangling pointer.
This is fixed by using QPointer in galagoNotifications (instead of in ::close,
because it will initiate a dbus call and others may be still willing to wait
for finished signal).
If KNotification is already deleted, don't call other related functions on this
KNotification.
- watcher is safer to call deleteLater on watcher first.
Diffs
-
src/notifybypopup.cpp 95937fc
Diff: https://git.reviewboard.kde.org/r/122393/diff/
Testing
---
Above code doesn't crash anymore.
Thanks,
Xuetian Weng
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122393: Fix dangling pointer and possible memory leak in knotifications
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/#review75245
---
Ship it!
Ship It!
- Martin Klapetek
On Feb. 2, 2015, 10:35 p.m., Xuetian Weng wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122393/
---
(Updated Feb. 2, 2015, 10:35 p.m.)
Review request for KDE Frameworks and Martin Klapetek.
Bugs: 342752
https://bugs.kde.org/show_bug.cgi?id=342752
Repository: knotifications
Description
---
- Follow code would crash with current code, because notificationQueue may
contain dangling pointer to KNotification.
#include QApplication
#include KNotification
int main(int argc, char* argv[]) {
QApplication app(argc,argv);
do {
KNotification a(NewMessage);
a.setComponentName(kwrited); // just a random component for test
a.sendEvent();
} while(0);
app.exec();
return 0;
}
This is fixed by remove item from queue in ::close.
- 342752 might happen because galagoNotifications may contain dangling
pointer.
This is fixed by using QPointer in galagoNotifications (instead of in
::close, because it will initiate a dbus call and others may be still willing
to wait for finished signal).
If KNotification is already deleted, don't call other related functions on
this KNotification.
- watcher is safer to call deleteLater on watcher first.
Diffs
-
src/notifybypopup.cpp 95937fc
Diff: https://git.reviewboard.kde.org/r/122393/diff/
Testing
---
Above code doesn't crash anymore.
Thanks,
Xuetian Weng
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122300: Fix quoteArgs for spaces that are not the regular space
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122300/ --- (Updated Feb. 2, 2015, 10:46 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, David Faure, Eike Hein, and Michael Pyne. Bugs: 343380 https://bugs.kde.org/show_bug.cgi?id=343380 Repository: kcoreaddons Description --- We need to also quote spaces like the Unicode character 'IDEOGRAPHIC SPACE' (U+3000) in KShell::quoteArg since KShell::splitArgs is using QChar::isSpace to decide when a new argument starts, without the quoting, one argument will get turned into two Diffs - autotests/kshelltest.cpp 451bbe9 src/lib/util/kshell_unix.cpp e0e884f Diff: https://git.reviewboard.kde.org/r/122300/diff/ Testing --- New test passes, bug 343380 is fixed Thanks, Albert Astals Cid ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122396: Fix knotifyconfig leak
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122396/ --- Review request for KDE Frameworks and Martin Klapetek. Repository: knotifications Description --- knotifyconfig in notify would not be free'd in many case. This patch pass const reference instead of pointer to plugin. And add a copy constructor to KNotifyConfig, so it's not possible to leak anymore. Though API/ABI is changed, knotifyconfig.h is never installed before, which also makes knotificationplugin somewhat useless. And seems there is no knotificationplugin user yet. Diffs - src/notifybypopup.h a605a77 src/notifybypopup.cpp 2f08146 src/notifybytaskbar.h 0908787 src/notifybytaskbar.cpp b52e180 src/knotificationmanager.cpp 3cf637c src/knotificationplugin.h 3438e7d src/knotificationplugin.cpp 7315c17 src/knotifyconfig.h fae17cb src/knotifyconfig.cpp af6be92 src/notifybyaudio.h 9a2d531 src/notifybyaudio.cpp d7090ac src/notifybyexecute.h ae362e3 src/notifybyexecute.cpp b5d437a src/notifybyktts.h 43756f7 src/notifybyktts.cpp a2a15a9 src/notifybylogfile.h 6ea2850 src/notifybylogfile.cpp ca3faf9 Diff: https://git.reviewboard.kde.org/r/122396/diff/ Testing --- Works. Thanks, Xuetian Weng ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122330: Associate *.qmltypes and *.qmlproject files with the text/x-qml mime type
On Feb. 1, 2015, 8:32 p.m., David Faure wrote: -1 for not submitting this upstream for shared-mime-info. I took care of it, it's committed, will be in s-m-i 1.4. Could we add a comment to this file then, telling people to work on upstream smi instead then? - Milian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122330/#review75179 --- On Feb. 1, 2015, 7:28 p.m., Denis Steckelmacher wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122330/ --- (Updated Feb. 1, 2015, 7:28 p.m.) Review request for KDE Frameworks, kdelibs and KDevelop. Repository: kcoreaddons Description --- The KDevelop QML/JS plugin sometimes needs to parse .qmltypes files (in order to list the content of installed QML modules, for instance), but KDevelop requires that files parsed using the QML/JS plugin have the text/x-qml mime type. Because .qmltypes and .qmlproject files are valid QML files (they follow the standard QML syntax), this patch proposes to add these extensions to the ones associated with the text/x-qml mime type. Diffs - src/mimetypes/kde5.xml cc9f71e Diff: https://git.reviewboard.kde.org/r/122330/diff/ Testing --- Installing kcoreaddons and updating the system MIME database allows KDevelop to detect that files with the .qmltypes and .qmlproject extensions have to be parsed using the QML/JS language support plugin. This allows QML files to use QML modules installed system-wide (and fixes the unit tests of kdev-qmljs). Thanks, Denis Steckelmacher ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122383: Remove internal copy of KXmlRpcClient
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122383/ --- Review request for KDE Frameworks, Plasma and Rohan Garg. Repository: plasma-workspace Description --- The internal copy isn't needed as soon as KXmlRpcClient becomes a framework. Diffs - drkonqi/internalkxmlrpcclient/KF5XmlRpcClientPrivateConfig.cmake.in 8f99a29 drkonqi/internalkxmlrpcclient/README d8b6cd2 drkonqi/internalkxmlrpcclient/autotests/CMakeLists.txt f737d14 drkonqi/internalkxmlrpcclient/autotests/testclient.h e4897a3 drkonqi/internalkxmlrpcclient/autotests/testclient.cpp f10b20a drkonqi/internalkxmlrpcclient/autotests/testquery.h 7957af6 drkonqi/internalkxmlrpcclient/autotests/testquery.cpp f43a7ff drkonqi/internalkxmlrpcclient/src/CMakeLists.txt 884599e drkonqi/internalkxmlrpcclient/src/Messages.sh 059f34d drkonqi/internalkxmlrpcclient/src/client.h 60866da drkonqi/internalkxmlrpcclient/src/client.cpp 779a01a drkonqi/internalkxmlrpcclient/src/query.h cd6b786 drkonqi/internalkxmlrpcclient/src/query.cpp ff18e24 drkonqi/tests/bugzillalibtest/bugzillalibtest.cpp 83d299a CMakeLists.txt e4d2b2d drkonqi/CMakeLists.txt d851d84 drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 12d9626 drkonqi/internalkxmlrpcclient/CMakeLists.txt 276aeaa drkonqi/internalkxmlrpcclient/COPYING.BSD cca2a5c Diff: https://git.reviewboard.kde.org/r/122383/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Change in kio[master]: Check we're a QGuiApp before using ClipboardUpdater.
Eike Hein has uploaded a new change for review.
https://gerrit.vesnicky.cesnet.cz/r/351
Change subject: Check we're a QGuiApp before using ClipboardUpdater.
..
Check we're a QGuiApp before using ClipboardUpdater.
Matches other callsites in the file and prevents a known crash
in kio_desktop.
BUG:343443
Change-Id: Ifd0e83f6d87db6c1c5200af96e31b93636365c7c
---
M src/widgets/jobuidelegate.cpp
1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://gerrit.vesnicky.cesnet.cz:29418/kio refs/changes/51/351/1
diff --git a/src/widgets/jobuidelegate.cpp b/src/widgets/jobuidelegate.cpp
index 775964e..9487dc6 100644
--- a/src/widgets/jobuidelegate.cpp
+++ b/src/widgets/jobuidelegate.cpp
@@ -368,7 +368,9 @@
void KIO::JobUiDelegate::updateUrlInClipboard(const QUrl src, const QUrl
dest)
{
-KIO::ClipboardUpdater::update(src, dest);
+if (qobject_castQGuiApplication *(qApp) != NULL) {
+KIO::ClipboardUpdater::update(src, dest);
+}
}
class KIOWidgetJobUiDelegateFactory : public KIO::JobUiDelegateFactory
--
To view, visit https://gerrit.vesnicky.cesnet.cz/r/351
To unsubscribe, visit https://gerrit.vesnicky.cesnet.cz/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifd0e83f6d87db6c1c5200af96e31b93636365c7c
Gerrit-PatchSet: 1
Gerrit-Project: kio
Gerrit-Branch: master
Gerrit-Owner: Eike Hein h...@kde.org
Gerrit-Reviewer: Sysadmin Testing Account n...@kde.org
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122383: Remove internal copy of KXmlRpcClient
On Feb. 2, 2015, 7:21 p.m., David Edmundson wrote: +1 as soon as it's a framework +1 to what David said - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122383/#review75229 --- On Feb. 2, 2015, 4:37 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122383/ --- (Updated Feb. 2, 2015, 4:37 p.m.) Review request for KDE Frameworks, Plasma and Rohan Garg. Repository: plasma-workspace Description --- The internal copy isn't needed as soon as KXmlRpcClient becomes a framework. Diffs - drkonqi/internalkxmlrpcclient/KF5XmlRpcClientPrivateConfig.cmake.in 8f99a29 drkonqi/internalkxmlrpcclient/README d8b6cd2 drkonqi/internalkxmlrpcclient/autotests/CMakeLists.txt f737d14 drkonqi/internalkxmlrpcclient/autotests/testclient.h e4897a3 drkonqi/internalkxmlrpcclient/autotests/testclient.cpp f10b20a drkonqi/internalkxmlrpcclient/autotests/testquery.h 7957af6 drkonqi/internalkxmlrpcclient/autotests/testquery.cpp f43a7ff drkonqi/internalkxmlrpcclient/src/CMakeLists.txt 884599e drkonqi/internalkxmlrpcclient/src/Messages.sh 059f34d drkonqi/internalkxmlrpcclient/src/client.h 60866da drkonqi/internalkxmlrpcclient/src/client.cpp 779a01a drkonqi/internalkxmlrpcclient/src/query.h cd6b786 drkonqi/internalkxmlrpcclient/src/query.cpp ff18e24 drkonqi/tests/bugzillalibtest/bugzillalibtest.cpp 83d299a CMakeLists.txt e4d2b2d drkonqi/CMakeLists.txt d851d84 drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 12d9626 drkonqi/internalkxmlrpcclient/CMakeLists.txt 276aeaa drkonqi/internalkxmlrpcclient/COPYING.BSD cca2a5c Diff: https://git.reviewboard.kde.org/r/122383/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build became unstable: plasma-framework_master_qt5 » All,LINBUILDER #985
See http://build.kde.org/job/plasma-framework_master_qt5/Variation=All,label=LINBUILDER/985/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122383: Remove internal copy of KXmlRpcClient
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122383/#review75229 --- +1 as soon as it's a framework - David Edmundson On Feb. 2, 2015, 3:37 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122383/ --- (Updated Feb. 2, 2015, 3:37 p.m.) Review request for KDE Frameworks, Plasma and Rohan Garg. Repository: plasma-workspace Description --- The internal copy isn't needed as soon as KXmlRpcClient becomes a framework. Diffs - drkonqi/internalkxmlrpcclient/KF5XmlRpcClientPrivateConfig.cmake.in 8f99a29 drkonqi/internalkxmlrpcclient/README d8b6cd2 drkonqi/internalkxmlrpcclient/autotests/CMakeLists.txt f737d14 drkonqi/internalkxmlrpcclient/autotests/testclient.h e4897a3 drkonqi/internalkxmlrpcclient/autotests/testclient.cpp f10b20a drkonqi/internalkxmlrpcclient/autotests/testquery.h 7957af6 drkonqi/internalkxmlrpcclient/autotests/testquery.cpp f43a7ff drkonqi/internalkxmlrpcclient/src/CMakeLists.txt 884599e drkonqi/internalkxmlrpcclient/src/Messages.sh 059f34d drkonqi/internalkxmlrpcclient/src/client.h 60866da drkonqi/internalkxmlrpcclient/src/client.cpp 779a01a drkonqi/internalkxmlrpcclient/src/query.h cd6b786 drkonqi/internalkxmlrpcclient/src/query.cpp ff18e24 drkonqi/tests/bugzillalibtest/bugzillalibtest.cpp 83d299a CMakeLists.txt e4d2b2d drkonqi/CMakeLists.txt d851d84 drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 12d9626 drkonqi/internalkxmlrpcclient/CMakeLists.txt 276aeaa drkonqi/internalkxmlrpcclient/COPYING.BSD cca2a5c Diff: https://git.reviewboard.kde.org/r/122383/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Change in kio[master]: Change how UDSEntry load and save parsing is handled.
Mark Gaiser has uploaded a new change for review.
https://gerrit.vesnicky.cesnet.cz/r/352
Change subject: Change how UDSEntry load and save parsing is handled.
..
Change how UDSEntry load and save parsing is handled.
This isn't faster or slower then before (benchmarks showed that it's about
equally in speed as before).
This patch is preperation work for more optimizations in SlaveBase. That will
write directly in a buffer
using similar methods as UDSEntry does now. UDSEntry is changed to be
consistent with that approach.
CHANGELOG: Change how UDSEntry load and save parsing is handled
Change-Id: I50af82e17f9fdd73e77d9e98411cf54fada2a962
---
M autotests/udsentrytest.cpp
M src/core/udsentry.cpp
2 files changed, 31 insertions(+), 17 deletions(-)
git pull ssh://gerrit.vesnicky.cesnet.cz:29418/kio refs/changes/52/352/1
diff --git a/autotests/udsentrytest.cpp b/autotests/udsentrytest.cpp
index 04e072e..066fd86 100644
--- a/autotests/udsentrytest.cpp
+++ b/autotests/udsentrytest.cpp
@@ -183,17 +183,21 @@
{
QDataStream stream(data, QIODevice::WriteOnly);
foreach (const QVectorUDSTestField testCase, testCases) {
-stream testCase.count();
+const int size = testCase.count();
+stream.writeRawData(reinterpret_castconst char*(size),
sizeof(int));
foreach (const UDSTestField field, testCase) {
uint uds = field.m_uds;
-stream uds;
+stream.writeRawData(reinterpret_castconst char*(uds),
sizeof(uint));
if (uds KIO::UDSEntry::UDS_STRING) {
-stream field.m_string;
+const QString str = field.m_string;
+int size = str.size();
+stream.writeRawData(reinterpret_castconst char*(size),
sizeof(int));
+stream.writeRawData(reinterpret_castconst
char*(str.utf16()), sizeof(ushort) * size);
} else {
Q_ASSERT(uds KIO::UDSEntry::UDS_NUMBER);
-stream field.m_long;
+stream.writeRawData(reinterpret_castconst
char*(field.m_long), sizeof(long long));
}
}
}
diff --git a/src/core/udsentry.cpp b/src/core/udsentry.cpp
index 38e40c5..d92b3fd 100644
--- a/src/core/udsentry.cpp
+++ b/src/core/udsentry.cpp
@@ -188,16 +188,19 @@
const QVectorField fields = a.d-fields;
const int size = udsIndexes.size();
-s size;
+s.writeRawData(reinterpret_castconst char*(size), sizeof(int));
for (int index = 0; index size; ++index) {
uint uds = udsIndexes.at(index);
-s uds;
+s.writeRawData(reinterpret_castconst char*(uds), sizeof(uint));
if (uds KIO::UDSEntry::UDS_STRING) {
-s fields.at(index).m_str;
+const QString str = fields.at(index).m_str;
+int size = str.size();
+s.writeRawData(reinterpret_castconst char*(size), sizeof(int));
+s.writeRawData(reinterpret_castconst char*(str.utf16()),
sizeof(ushort) * size);
} else if (uds KIO::UDSEntry::UDS_NUMBER) {
-s fields.at(index).m_long;
+s.writeRawData(reinterpret_castconst
char*(fields.at(index).m_long), sizeof(long long));
} else {
Q_ASSERT_X(false, KIO::UDSEntry, Found a field with an invalid
type);
}
@@ -211,8 +214,9 @@
QVectorField fields = a.d-fields;
QVectoruint udsIndexes = a.d-udsIndexes;
-quint32 size;
-s size;
+int size;
+s.readRawData(reinterpret_castchar*(size), sizeof(int));
+
fields.reserve(size);
udsIndexes.reserve(size);
@@ -220,21 +224,27 @@
// will often be the same for many entries in a row. Caching them
// permits to use implicit sharing to save memory.
static QVectorQString cachedStrings;
-if (quint32(cachedStrings.size()) size) {
+if (int(cachedStrings.size()) size) {
cachedStrings.resize(size);
}
-for (quint32 i = 0; i size; ++i) {
-quint32 uds;
-s uds;
+for (int i = 0; i size; ++i) {
+uint uds;
+s.readRawData(reinterpret_castchar*(uds), sizeof(uint));
+
udsIndexes.append(uds);
if (uds KIO::UDSEntry::UDS_STRING) {
// If the QString is the same like the one we read for the
// previous UDSEntry at the i-th position, use an implicitly
// shared copy of the same QString to save memory.
-QString buffer;
-s buffer;
+int length;
+s.readRawData(reinterpret_castchar*(length), sizeof(int));
+
+char tempBuffer[length * sizeof(ushort)];
+s.readRawData(tempBuffer, length * sizeof(ushort));
+
+QString buffer = QString::fromUtf16(reinterpret_castconst
ushort*(tempBuffer), length);
if (buffer !=
Jenkins build is back to stable : kservice_stable_qt5 #29
See http://build.kde.org/job/kservice_stable_qt5/29/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
