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