D29814: Fix segfault on no restart args

[Jiří Paleček]

jpalecek added a comment.


  In D29814#674013 <https://phabricator.kde.org/D29814#674013>, @dfaure wrote:
  
  > 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.
  
  
  No. It is a crash where `s_autoRestartCommandLine` was null. The actual crash happened in the logging code, but it is also used in `startProcess` and I think going for non-null `s_autoRestartCommandLine` is actually easier than faffing with null pointer checks and special casing in the signal handler code. Even with no restart arguments, you still need `argv` to hold the executable name to restart, so why not maintain it uniformly? It only costs a few bytes of heap.

INLINE COMMENTS

> dfaure wrote in kcrash.cpp:107
> Should this become 1 then? Otherwise the `for` loop won't delete that new `new char*[1]`

No. The invariant is that `s_autoRestartArgc` is the number of arguments, not counting the last `null`. The `new char*[1]` is deleted after the loop.

> dfaure wrote in kcrash.cpp:272
> insert(0) reads like prepend, but args is empty anyway, why not just use append?

Well I was thinking about `resize()`, then found there wasn't any. However, if we want to be clear, maybe `args = { QString() }` would be the best?

(In reality, the effect should be `args = { filePath }`, but that comes with the next line.)

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/649ccae9/attachment.htm>