Fix crash when trying to open the folder creation dialog #2990
Conversation
src/gui/foldercreationdialog.cpp
Outdated
| @@ -37,7 +37,7 @@ FolderCreationDialog::FolderCreationDialog(const QString &destination, QWidget * | |||
|
|
|||
| const QString suggestedFolderNamePrefix = QObject::tr("New folder"); | |||
|
|
|||
| const auto newFolderFullPath = _destination + "/" + suggestedFolderNamePrefix; | |||
| const QString newFolderFullPath = _destination + "/" + suggestedFolderNamePrefix; | |||
There was a problem hiding this comment.
@FlexW I am a bit confused about this because newFolderFullPath is then just passed to QDir(const QString& path) that should result in a similar implicit conversion from QStringBuilder to QString. But, if this works, let's move it forward.
Maybe @er-vin has some idea why this is happening?
Thread 1 "nextcloud" received signal SIGSEGV, Segmentation fault.
QString::size (this=0x5555567cfb50) at /usr/include/qt/QtCore/qstring.h:277
277 inline int size() const { return d->size; }
There was a problem hiding this comment.
The problem is that QStringBuilder is lazy evaluated and that the temporaries used in the expression (_destination, "/" and suggestedFolderNamePrefix) are long gone when the line of the QDir ctor is reached and the conversion to QString occurs.
BTW, it was here before but it should be QLatin1Char('/') instead of "/".
FlexW
force-pushed
the
bugfix/fix-create-folder-dialog-crash
branch
from
7f7b319
to
3ab717e
Compare
src/gui/foldercreationdialog.cpp
Outdated
| @@ -37,7 +40,9 @@ FolderCreationDialog::FolderCreationDialog(const QString &destination, QWidget * | |||
|
|
|||
| const QString suggestedFolderNamePrefix = QObject::tr("New folder"); | |||
|
|
|||
| const auto newFolderFullPath = _destination + "/" + suggestedFolderNamePrefix; | |||
| qInfo(lcFolderCreationDialog) << "Before setting newFolderNameEdit"; | |||
There was a problem hiding this comment.
Do we really want to introduce logs just for that? I mean they don't bring much value.
src/gui/foldercreationdialog.cpp
Outdated
| @@ -37,7 +37,7 @@ FolderCreationDialog::FolderCreationDialog(const QString &destination, QWidget * | |||
|
|
|||
| const QString suggestedFolderNamePrefix = QObject::tr("New folder"); | |||
|
|
|||
| const auto newFolderFullPath = _destination + "/" + suggestedFolderNamePrefix; | |||
| const QString newFolderFullPath = _destination + "/" + suggestedFolderNamePrefix; | |||
There was a problem hiding this comment.
The problem is that QStringBuilder is lazy evaluated and that the temporaries used in the expression (_destination, "/" and suggestedFolderNamePrefix) are long gone when the line of the QDir ctor is reached and the conversion to QString occurs.
BTW, it was here before but it should be QLatin1Char('/') instead of "/".
The bug does seem to just appear in special compiler constellations. We're unsure why this fix works. To better see if this fix works or if crashes still occur, we added some logging. Signed-off-by: Felix Weilbach <felix.weilbach@nextcloud.com>
FlexW
force-pushed
the
bugfix/fix-create-folder-dialog-crash
branch
from
3ab717e
to
f4853da
Compare
|
Btw: I wonder if we should not remove that dialog. I can hardly image that someone uses it. Since most of the time you will create folder/files from your Explorer/Nautilus/Terminal. |
|
AppImage file: Nextcloud-PR-2990-f4853da2ab73ef7deb9b3c47028587a05ae9a117-x86_64.AppImage |
There's still one case (and that's why @allexzander introduced that dialog in the first place): when you want to create E2EE folders. Without this you're forced to go to the file manager, then the settings dialog. With this tiny feature you can then do everything through the settings dialog. |
@FlexW The thing is, this was a feature request. So, it would be a shame to remove a feature right after adding it ;) |
|
@er-vin @allexzander Thanks for the explanation :) That makes sense. |
Signed-off-by: Felix Weilbach felix.weilbach@nextcloud.com