Apparently it's possible that there are no arguments, especially
if called during teardown from ~QApplication.
Details
Diff Detail
- Repository
- R626 QtCurve
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 14374 Build 14392: arc lint + arc unit
Didn't know this can happen and lgtm.
Idem (2x).
Out of curiosity: how do you invoke pinentry-qt to trigger the crash, where does the it occur, and what does qApp->arguments()[0] return if there are no arguments? FWIW my own copy of pinentry-qt starts and exits fine, but I don't know how to make it pop up a GUI.
Before I give the green light too, do you also get a crash when you use qApp->arguments().at(0)? This method doesn't work exactly the same way as the [] operator does.
Of course we could also just query qApp->applicationName() ...
Just pinentry-qt and then BYE. Backtrace and more info on https://bugzilla.opensuse.org/show_bug.cgi?id=1141883
The invalid read "0 bytes after a block of 16 bytes" is visible with valgrind.
Before I give the green light too, do you also get a crash when you use qApp->arguments().at(0)? This method doesn't work exactly the same way as the [] operator does.
I assume so, as arguments is empty both would be invalid.
Of course we could also just query qApp->applicationName() ...
Just `pinentry-qt` and then `BYE`. Backtrace and more info on https://bugzilla.opensuse.org/show_bug.cgi?id=1141883
That backtrace isn't very helpful, since missing most line numbers.
> pinentry-qt --debug org.kde.kwindowsystem: Loaded plugin "/opt/local/share/qt5/plugins/kf5/org.kde.kwindowsystem.platforms/KF5WindowSystemX11Plugin.so" for platform "xcb" OK Pleased to meet you BYE OK closing connection
I assume so, as arguments is empty both would be invalid.
Not necessarily, QStringList::at(0) could return an empty QString instance if the list is empty.
Your comment https://bugzilla.opensuse.org/show_bug.cgi?id=1141883#c17 does make wonder if OpenSuse has the latest QtCurve version (or more specifically, if the proposed change is the only fix/workaround for the issue in the latest QtCurve version from git?
Not necessarily, QStringList::at(0) could return an empty QString instance if the list is empty.
No. Also that's basically impossible from the return type (a reference).
Tumbleweed has the latest released version and the crash happens also with git master.
While I narrowed down the root cause of argc suddenly being empty to a dangling pointer in pinentry, I'd still say that this patch is correct (or might be extended to connectDBus() as well).
OK.
Would you mind checking if using QCoreApplication::applicationName() would be an alternative? I'd do it myself but since I cannot reproduce your issue I can't answer the question fully.
There's another alternative: caching the information ourselves. That would remove all possibilities of race conditions during global destruction when we can no longer rely on Qt's own data.
or might be extended to connectDBus() as well
Do you mean (or have any indication that) arguments() can be empty when the application is starting up?
Hm, totally missed the notification for this...
At worst it would return a null QString, so should work fine, yes.
There's another alternative: caching the information ourselves. That would remove all possibilities of race conditions during global destruction when we can no longer rely on Qt's own data.
or might be extended to connectDBus() as well
Do you mean (or have any indication that) arguments() can be empty when the application is starting up?
Unless argc is 0 when constructing the QCoreApplication, I don't think so.
Go figure... I had exactly this kind of crash the other day, despite your proposed change.
Turns out this can still happen in applications that do not destroy the Q*Application instance before exitting or returning from main(). Which BTW is the official way to do things, even if it isn't documented explicitly.
Since that is something we cannot control I have indeed decided to use cached information. The application name was already being cached in a global variable that's used for application-specific behaviour, I guess you could say it should have been used for that throughout QtCurve.