Re: Review Request 116087: remove usage of strlcpy
On March 16, 2014, 9:12 a.m., David Faure wrote: src/kcrash.cpp, line 56 https://git.reviewboard.kde.org/r/116087/diff/3/?file=254139#file254139line56 I just realized that this requires qpa-private headers, which is a problem (breaks compat when upgrading Qt, too). See k-f-d thread kguiaddons uses qpa headers?. We could add a QX11Info::startupId() instead? Makes me wonder how startup notification is supposed to work on wayland though - and whether I should resurrect the idea of a DBus-based startup notification at the upcoming freedesktop meeting. Alex Merry wrote: Kevin Krammer queried just how private the private headers really are in that thread... WRT the D-Bus startup notification, I'm greatly in favour. I think Martin Gräßlin is as well, from previous email/reviewboard threads. Alexander Richardson wrote: Should I then commit the first version of this patch until we have a proper solution? David Faure wrote: We just got rid of qpa private headers in kguiaddons, I'm not sure that creating the same issue in kcrash is a good idea, especially since the code works right now, without that. We could instead add QX11Info::startupId() and ifdef based on the Qt version? Alexander Richardson wrote: I mean the version where the QByteArray is just stored on the stack to increase the refcount. Adding QX11Info::startupId() is obviously the better solution, but that will only be useful once we depend on Qt 5.4 Ah sorry, I didn't see first version when reading too fast. OK, let's go for that then. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/#review53028 --- On March 15, 2014, 3:31 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/ --- (Updated March 15, 2014, 3:31 p.m.) Review request for KDE Frameworks and David Faure. Repository: kcrash Description --- remove usage of strlcpy We can get the startupId from the QPlatformNativeInterface. Additionally this means we no longer need to link to KWindowSystem. REVIEW: 116087 Diffs - src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e KF5CrashConfig.cmake.in dcde84376e7ea69e53a26dc2bcba8137ee28a2a4 CMakeLists.txt b9a6f441abed3c88bf428869c30ef5aebd72fc74 src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8 Diff: https://git.reviewboard.kde.org/r/116087/diff/ Testing --- Compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: remove usage of strlcpy
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/#review53174 --- This review has been submitted with commit 0aa82c64c0c8c9c2e6d141ecadab418f8d8911ed by Alex Richardson to branch master. - Commit Hook On March 15, 2014, 3:31 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/ --- (Updated March 15, 2014, 3:31 p.m.) Review request for KDE Frameworks and David Faure. Repository: kcrash Description --- remove usage of strlcpy We can get the startupId from the QPlatformNativeInterface. Additionally this means we no longer need to link to KWindowSystem. REVIEW: 116087 Diffs - src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e KF5CrashConfig.cmake.in dcde84376e7ea69e53a26dc2bcba8137ee28a2a4 CMakeLists.txt b9a6f441abed3c88bf428869c30ef5aebd72fc74 src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8 Diff: https://git.reviewboard.kde.org/r/116087/diff/ Testing --- Compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: remove usage of strlcpy
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/ --- (Updated March 17, 2014, 12:25 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Repository: kcrash Description --- remove usage of strlcpy We can get the startupId from the QPlatformNativeInterface. Additionally this means we no longer need to link to KWindowSystem. REVIEW: 116087 Diffs - src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e KF5CrashConfig.cmake.in dcde84376e7ea69e53a26dc2bcba8137ee28a2a4 CMakeLists.txt b9a6f441abed3c88bf428869c30ef5aebd72fc74 src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8 Diff: https://git.reviewboard.kde.org/r/116087/diff/ Testing --- Compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: remove usage of strlcpy
On March 16, 2014, 9:12 a.m., David Faure wrote: src/kcrash.cpp, line 56 https://git.reviewboard.kde.org/r/116087/diff/3/?file=254139#file254139line56 I just realized that this requires qpa-private headers, which is a problem (breaks compat when upgrading Qt, too). See k-f-d thread kguiaddons uses qpa headers?. We could add a QX11Info::startupId() instead? Makes me wonder how startup notification is supposed to work on wayland though - and whether I should resurrect the idea of a DBus-based startup notification at the upcoming freedesktop meeting. Kevin Krammer queried just how private the private headers really are in that thread... WRT the D-Bus startup notification, I'm greatly in favour. I think Martin Gräßlin is as well, from previous email/reviewboard threads. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/#review53028 --- On March 15, 2014, 3:31 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/ --- (Updated March 15, 2014, 3:31 p.m.) Review request for KDE Frameworks and David Faure. Repository: kcrash Description --- remove usage of strlcpy We can get the startupId from the QPlatformNativeInterface. Additionally this means we no longer need to link to KWindowSystem. REVIEW: 116087 Diffs - src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e KF5CrashConfig.cmake.in dcde84376e7ea69e53a26dc2bcba8137ee28a2a4 CMakeLists.txt b9a6f441abed3c88bf428869c30ef5aebd72fc74 src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8 Diff: https://git.reviewboard.kde.org/r/116087/diff/ Testing --- Compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: remove usage of strlcpy
On March 16, 2014, 10:12 a.m., David Faure wrote: src/kcrash.cpp, line 56 https://git.reviewboard.kde.org/r/116087/diff/3/?file=254139#file254139line56 I just realized that this requires qpa-private headers, which is a problem (breaks compat when upgrading Qt, too). See k-f-d thread kguiaddons uses qpa headers?. We could add a QX11Info::startupId() instead? Makes me wonder how startup notification is supposed to work on wayland though - and whether I should resurrect the idea of a DBus-based startup notification at the upcoming freedesktop meeting. Alex Merry wrote: Kevin Krammer queried just how private the private headers really are in that thread... WRT the D-Bus startup notification, I'm greatly in favour. I think Martin Gräßlin is as well, from previous email/reviewboard threads. Should I then commit the first version of this patch until we have a proper solution? - Alexander --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/#review53028 --- On March 15, 2014, 4:31 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/ --- (Updated March 15, 2014, 4:31 p.m.) Review request for KDE Frameworks and David Faure. Repository: kcrash Description --- remove usage of strlcpy We can get the startupId from the QPlatformNativeInterface. Additionally this means we no longer need to link to KWindowSystem. REVIEW: 116087 Diffs - src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e KF5CrashConfig.cmake.in dcde84376e7ea69e53a26dc2bcba8137ee28a2a4 CMakeLists.txt b9a6f441abed3c88bf428869c30ef5aebd72fc74 src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8 Diff: https://git.reviewboard.kde.org/r/116087/diff/ Testing --- Compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: remove usage of strlcpy
On March 16, 2014, 9:12 a.m., David Faure wrote: src/kcrash.cpp, line 56 https://git.reviewboard.kde.org/r/116087/diff/3/?file=254139#file254139line56 I just realized that this requires qpa-private headers, which is a problem (breaks compat when upgrading Qt, too). See k-f-d thread kguiaddons uses qpa headers?. We could add a QX11Info::startupId() instead? Makes me wonder how startup notification is supposed to work on wayland though - and whether I should resurrect the idea of a DBus-based startup notification at the upcoming freedesktop meeting. Alex Merry wrote: Kevin Krammer queried just how private the private headers really are in that thread... WRT the D-Bus startup notification, I'm greatly in favour. I think Martin Gräßlin is as well, from previous email/reviewboard threads. Alexander Richardson wrote: Should I then commit the first version of this patch until we have a proper solution? We just got rid of qpa private headers in kguiaddons, I'm not sure that creating the same issue in kcrash is a good idea, especially since the code works right now, without that. We could instead add QX11Info::startupId() and ifdef based on the Qt version? - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/#review53028 --- On March 15, 2014, 3:31 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/ --- (Updated March 15, 2014, 3:31 p.m.) Review request for KDE Frameworks and David Faure. Repository: kcrash Description --- remove usage of strlcpy We can get the startupId from the QPlatformNativeInterface. Additionally this means we no longer need to link to KWindowSystem. REVIEW: 116087 Diffs - src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e KF5CrashConfig.cmake.in dcde84376e7ea69e53a26dc2bcba8137ee28a2a4 CMakeLists.txt b9a6f441abed3c88bf428869c30ef5aebd72fc74 src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8 Diff: https://git.reviewboard.kde.org/r/116087/diff/ Testing --- Compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: remove usage of strlcpy
On March 16, 2014, 10:12 a.m., David Faure wrote: src/kcrash.cpp, line 56 https://git.reviewboard.kde.org/r/116087/diff/3/?file=254139#file254139line56 I just realized that this requires qpa-private headers, which is a problem (breaks compat when upgrading Qt, too). See k-f-d thread kguiaddons uses qpa headers?. We could add a QX11Info::startupId() instead? Makes me wonder how startup notification is supposed to work on wayland though - and whether I should resurrect the idea of a DBus-based startup notification at the upcoming freedesktop meeting. Alex Merry wrote: Kevin Krammer queried just how private the private headers really are in that thread... WRT the D-Bus startup notification, I'm greatly in favour. I think Martin Gräßlin is as well, from previous email/reviewboard threads. Alexander Richardson wrote: Should I then commit the first version of this patch until we have a proper solution? David Faure wrote: We just got rid of qpa private headers in kguiaddons, I'm not sure that creating the same issue in kcrash is a good idea, especially since the code works right now, without that. We could instead add QX11Info::startupId() and ifdef based on the Qt version? I mean the version where the QByteArray is just stored on the stack to increase the refcount. Adding QX11Info::startupId() is obviously the better solution, but that will only be useful once we depend on Qt 5.4 - Alexander --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/#review53028 --- On March 15, 2014, 4:31 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/ --- (Updated March 15, 2014, 4:31 p.m.) Review request for KDE Frameworks and David Faure. Repository: kcrash Description --- remove usage of strlcpy We can get the startupId from the QPlatformNativeInterface. Additionally this means we no longer need to link to KWindowSystem. REVIEW: 116087 Diffs - src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e KF5CrashConfig.cmake.in dcde84376e7ea69e53a26dc2bcba8137ee28a2a4 CMakeLists.txt b9a6f441abed3c88bf428869c30ef5aebd72fc74 src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8 Diff: https://git.reviewboard.kde.org/r/116087/diff/ Testing --- Compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: remove usage of strlcpy
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/ --- (Updated March 15, 2014, 4:31 p.m.) Review request for KDE Frameworks and David Faure. Repository: kcrash Description --- remove usage of strlcpy We can get the startupId from the QPlatformNativeInterface. Additionally this means we no longer need to link to KWindowSystem. REVIEW: 116087 Diffs (updated) - src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e KF5CrashConfig.cmake.in dcde84376e7ea69e53a26dc2bcba8137ee28a2a4 CMakeLists.txt b9a6f441abed3c88bf428869c30ef5aebd72fc74 src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8 Diff: https://git.reviewboard.kde.org/r/116087/diff/ Testing --- Compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: remove usage of strlcpy
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/ --- (Updated March 14, 2014, 1:38 p.m.) Review request for KDE Frameworks and David Faure. Summary (updated) - remove usage of strlcpy Repository: kcrash Description (updated) --- remove usage of strlcpy We can get the startupId from the QPlatformNativeInterface. Additionally this means we no longer need to link to KWindowSystem. REVIEW: 116087 Diffs (updated) - CMakeLists.txt b9a6f441abed3c88bf428869c30ef5aebd72fc74 KF5CrashConfig.cmake.in dcde84376e7ea69e53a26dc2bcba8137ee28a2a4 src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8 src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d Diff: https://git.reviewboard.kde.org/r/116087/diff/ Testing --- Compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116087: remove usage of strlcpy
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/#review52938 --- src/kcrash.cpp https://git.reviewboard.kde.org/r/116087/#comment37256 A stylistic point: I think removing the blank lines makes it harder to read src/kcrash.cpp https://git.reviewboard.kde.org/r/116087/#comment37257 I think it might be worth documenting (in a comment) the assumption that the main/GUI thread is stopped, and so no windows will be created, and so the startupid won't go away. - Alex Merry On March 14, 2014, 12:38 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116087/ --- (Updated March 14, 2014, 12:38 p.m.) Review request for KDE Frameworks and David Faure. Repository: kcrash Description --- remove usage of strlcpy We can get the startupId from the QPlatformNativeInterface. Additionally this means we no longer need to link to KWindowSystem. REVIEW: 116087 Diffs - CMakeLists.txt b9a6f441abed3c88bf428869c30ef5aebd72fc74 KF5CrashConfig.cmake.in dcde84376e7ea69e53a26dc2bcba8137ee28a2a4 src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e src/config-strlcpy.h.cmake 5d9163d7a60d89b9792afcdf498af6425a9038a8 src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d Diff: https://git.reviewboard.kde.org/r/116087/diff/ Testing --- Compiles Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
