D29814: Fix segfault on no restart args
David Faure
noreply at phabricator.kde.org
Wed May 27 22:40:00 BST 2020
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
I'm a bit confused by the bug this is fixing. Surely this doesn't happen to all cases of crashes without autorestart enabled??
Also, it sounds like a null check might be enough.
INLINE COMMENTS
> kcrash.cpp:107
> static char *s_appPath = nullptr;
> static int s_autoRestartArgc = 0;
> +static char **s_autoRestartCommandLine = new char*[1]{ nullptr };
Should this become 1 then? Otherwise the `for` loop won't delete that new `new char*[1]`
> kcrash.cpp:272
> + if (args.isEmpty()) // edge case: tst_QX11Info::startupId does QApplication app(argc, nullptr)...
> + args.insert(0, QString());
> +
insert(0) reads like prepend, but args is empty anyway, why not just use append?
> kcrash.cpp:275
> + args[0] = filePath; // replace argv[0] with full path above
> + for (int arg = 0; arg < s_autoRestartArgc; arg++)
> + delete [] s_autoRestartCommandLine[arg];
We use { ... } braces also around single-line statements in KF5.
Can you do the same for the `if` above too?
REPOSITORY
R285 KCrash
REVISION DETAIL
https://phabricator.kde.org/D29814
To: jpalecek, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200527/41145f76/attachment-0001.htm>
More information about the Kde-frameworks-devel
mailing list