> 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