> On March 1, 2014, 4:17 p.m., David Faure wrote:
> > Hmm, this might be equivalent, but all it means is that the orig code was
> > wrong.
> >
> > We should not make any memory allocations within the crash handler.
> >
> > So we should instead store the startup id as a const char* somewhere and
> > use strlcpy.
> >
> > Unless we can make sure that the call to startupId() will always just
> > return an existing QByteArray - but looking at the code, this doesn't seem
> > to be the case.
>
> Alex Merry wrote:
> Hrm... I think we're actually querying the wrong place anyway. We should
> be asking the xcb platform plugin, since that's what actually handles the
> startup notifications, since the demise of KApplication (perhaps that part of
> KStartupInfo's functionality should be stripped out?).
>
> Note that the platform plugin does always just return an existing
> QByteArray, so that should be fine. Which is good, because taking our own
> copy outside the handler would not work, as we would need to know when it was
> cleared.
>
> Alex Merry wrote:
> Actually, you don't get access to the QByteArray. You could do something
> like
>
> QPlatformNativeInterface *platform =
> QGuiApplication::platformNativeInterface();
> const char * startupId = reinterpret_cast<char
> *>(platform->nativeResourceForIntegration(QByteArrayLiteral("startupid")));
> if (startupId && *startupId) {
> argv[i++] = "--startupid";
> argv[i++] = startupId;
> }
>
> Since we're in the crash handler, is it safe to assume that the rest of
> the application is stopped, and so that string will never disappear from
> underneath us?
>
> Alexander Richardson wrote:
> I'll update the review to use this solution if someone else can confirm
> that this is safe. Even in multithreaded environments? Does the crash handler
> stop all threads?
>
> Kevin Ottens wrote:
> I think so yes, David could you confirm?
I'm not an expert on crash handling in a multithreaded application, but anyway,
threads do not call into the xcb plugin (it's not threadsafe, and it's a GUI
thing anyway). So yeah Alex Merry's suggestion sounds fine.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116087/#review51440
-----------------------------------------------------------
On March 12, 2014, 3:26 p.m., Alexander Richardson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116087/
> -----------------------------------------------------------
>
> (Updated March 12, 2014, 3:26 p.m.)
>
>
> Review request for KDE Frameworks and David Faure.
>
>
> Repository: kcrash
>
>
> Description
> -------
>
> remove usage of strlcpy
>
> That copy was actually unnecessary, incrementing the refcount on
> QByteArray and then calling constData() is enough
>
>
> Diffs
> -----
>
> src/kcrash.cpp 6c41022206130f186d52ddbb77afd58c429f368f
> src/strlcpy-fake.c 9b2dc68c466908d11370ba0c4bbe8d315962da5d
> src/CMakeLists.txt d19b97f98e057d5537cf0eb6a8e1997d2a24cb1e
> 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
