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