From a7b50c40a0af8e617b73f581c019d0f16f7d04d8 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Fri, 8 Sep 2023 16:25:44 +0200 Subject: [PATCH] macOS file dialog refactor: don't heap allocate string and string list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We always allocated them in the constructor function, and never tested them for nullptr, so just manage them as regular members. As a drive-by, apply const to read-only variables in relevant code. Pick-to: 6.6 Change-Id: If0a3ac8982582f2adf5187a3c0357f4da93467fb Reviewed-by: Tor Arne Vestbø --- .../platforms/cocoa/qcocoafiledialoghelper.mm | 64 +++++++++---------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm b/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm index 1b414d0771..4ddc8b9a23 100644 --- a/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm +++ b/src/plugins/platforms/cocoa/qcocoafiledialoghelper.mm @@ -58,9 +58,9 @@ typedef QSharedPointer SharedPointerFileDialogOptions; NSString *m_currentDirectory; SharedPointerFileDialogOptions m_options; - QString *m_currentSelection; - QStringList *m_nameFilterDropDownList; - QStringList *m_selectedNameFilter; + QString m_currentSelection; + QStringList m_nameFilterDropDownList; + QStringList m_selectedNameFilter; } - (instancetype)initWithAcceptMode:(const QString &)selectFile @@ -80,24 +80,24 @@ typedef QSharedPointer SharedPointerFileDialogOptions; m_helper = helper; - m_nameFilterDropDownList = new QStringList(m_options->nameFilters()); + m_nameFilterDropDownList = m_options->nameFilters(); QString selectedVisualNameFilter = m_options->initiallySelectedNameFilter(); - m_selectedNameFilter = new QStringList([self findStrippedFilterWithVisualFilterName:selectedVisualNameFilter]); + m_selectedNameFilter = [self findStrippedFilterWithVisualFilterName:selectedVisualNameFilter]; - QFileInfo sel(selectFile); + const QFileInfo sel(selectFile); if (sel.isDir() && !sel.isBundle()){ m_currentDirectory = [sel.absoluteFilePath().toNSString() retain]; - m_currentSelection = new QString; + m_currentSelection.clear(); } else { m_currentDirectory = [sel.absolutePath().toNSString() retain]; - m_currentSelection = new QString(sel.absoluteFilePath()); + m_currentSelection = sel.absoluteFilePath(); } [self createPopUpButton:selectedVisualNameFilter hideDetails:options->testOption(QFileDialogOptions::HideNameFilterDetails)]; [self createTextField]; [self createAccessory]; - m_panel.accessoryView = m_nameFilterDropDownList->size() > 1 ? m_accessoryView : nil; + m_panel.accessoryView = m_nameFilterDropDownList.size() > 1 ? m_accessoryView : nil; // -setAccessoryView: can result in -panel:directoryDidChange: // resetting our m_currentDirectory, set the delegate // here to make sure it gets the correct value. @@ -113,10 +113,6 @@ typedef QSharedPointer SharedPointerFileDialogOptions; - (void)dealloc { - delete m_nameFilterDropDownList; - delete m_selectedNameFilter; - delete m_currentSelection; - [m_panel orderOut:m_panel]; m_panel.accessoryView = nil; [m_popupButton release]; @@ -130,7 +126,7 @@ typedef QSharedPointer SharedPointerFileDialogOptions; - (bool)showPanel:(Qt::WindowModality) windowModality withParent:(QWindow *)parent { - QFileInfo info(*m_currentSelection); + const QFileInfo info(m_currentSelection); NSString *filepath = info.filePath().toNSString(); NSURL *url = [NSURL fileURLWithPath:filepath isDirectory:info.isDir()]; bool selectable = (m_options->acceptMode() == QFileDialogOptions::AcceptSave) @@ -184,7 +180,7 @@ typedef QSharedPointer SharedPointerFileDialogOptions; - (void)closePanel { - *m_currentSelection = QString::fromNSString(m_panel.URL.path).normalized(QString::NormalizationForm_C); + m_currentSelection = QString::fromNSString(m_panel.URL.path).normalized(QString::NormalizationForm_C); if (m_panel.sheet) [NSApp endSheet:m_panel]; @@ -202,7 +198,7 @@ typedef QSharedPointer SharedPointerFileDialogOptions; if (!filename.length) return NO; - QFileInfo fileInfo(QString::fromNSString(filename)); + const QFileInfo fileInfo(QString::fromNSString(filename)); // Always accept directories regardless of their names. // This also includes symlinks and aliases to directories. @@ -217,12 +213,12 @@ typedef QSharedPointer SharedPointerFileDialogOptions; return YES; } - QString qtFileName = fileInfo.fileName(); + const QString qtFileName = fileInfo.fileName(); // No filter means accept everything - bool nameMatches = m_selectedNameFilter->isEmpty(); + bool nameMatches = m_selectedNameFilter.isEmpty(); // Check if the current file name filter accepts the file: - for (int i = 0; !nameMatches && i < m_selectedNameFilter->size(); ++i) { - if (QDir::match(m_selectedNameFilter->at(i), qtFileName)) + for (int i = 0; !nameMatches && i < m_selectedNameFilter.size(); ++i) { + if (QDir::match(m_selectedNameFilter.at(i), qtFileName)) nameMatches = true; } if (!nameMatches) @@ -256,10 +252,10 @@ typedef QSharedPointer SharedPointerFileDialogOptions; - (void)setNameFilters:(const QStringList &)filters hideDetails:(BOOL)hideDetails { [m_popupButton removeAllItems]; - *m_nameFilterDropDownList = filters; + m_nameFilterDropDownList = filters; if (filters.size() > 0){ for (int i = 0; i < filters.size(); ++i) { - QString filter = hideDetails ? [self removeExtensions:filters.at(i)] : filters.at(i); + const QString filter = hideDetails ? [self removeExtensions:filters.at(i)] : filters.at(i); [m_popupButton.menu addItemWithTitle:filter.toNSString() action:nil keyEquivalent:@""]; } [m_popupButton selectItemAtIndex:0]; @@ -277,8 +273,8 @@ typedef QSharedPointer SharedPointerFileDialogOptions; Q_UNUSED(sender); if (!m_helper) return; - QString selection = m_nameFilterDropDownList->value([m_popupButton indexOfSelectedItem]); - *m_selectedNameFilter = [self findStrippedFilterWithVisualFilterName:selection]; + const QString selection = m_nameFilterDropDownList.value([m_popupButton indexOfSelectedItem]); + m_selectedNameFilter = [self findStrippedFilterWithVisualFilterName:selection]; [m_panel validateVisibleColumns]; [self updateProperties]; @@ -368,9 +364,9 @@ typedef QSharedPointer SharedPointerFileDialogOptions; return; if (m_panel.visible) { - QString selection = QString::fromNSString(m_panel.URL.path); - if (selection != *m_currentSelection) { - *m_currentSelection = selection; + const QString selection = QString::fromNSString(m_panel.URL.path); + if (selection != m_currentSelection) { + m_currentSelection = selection; emit m_helper->currentChanged(QUrl::fromLocalFile(selection)); } } @@ -410,7 +406,7 @@ typedef QSharedPointer SharedPointerFileDialogOptions; return nil; // panel:shouldEnableURL: does the file filtering for NSOpenPanel QStringList fileTypes; - for (const QString &filter : *m_selectedNameFilter) { + for (const QString &filter : std::as_const(m_selectedNameFilter)) { if (!filter.startsWith("*."_L1)) continue; @@ -457,10 +453,10 @@ typedef QSharedPointer SharedPointerFileDialogOptions; m_popupButton.target = self; m_popupButton.action = @selector(filterChanged:); - if (m_nameFilterDropDownList->size() > 0) { + if (!m_nameFilterDropDownList.isEmpty()) { int filterToUse = -1; - for (int i = 0; i < m_nameFilterDropDownList->size(); ++i) { - QString currentFilter = m_nameFilterDropDownList->at(i); + for (int i = 0; i < m_nameFilterDropDownList.size(); ++i) { + const QString currentFilter = m_nameFilterDropDownList.at(i); if (selectedFilter == currentFilter || (filterToUse == -1 && currentFilter.startsWith(selectedFilter))) filterToUse = i; @@ -474,9 +470,9 @@ typedef QSharedPointer SharedPointerFileDialogOptions; - (QStringList) findStrippedFilterWithVisualFilterName:(QString)name { - for (int i = 0; i < m_nameFilterDropDownList->size(); ++i) { - if (m_nameFilterDropDownList->at(i).startsWith(name)) - return QPlatformFileDialogHelper::cleanFilterList(m_nameFilterDropDownList->at(i)); + for (const QString ¤tFilter : std::as_const(m_nameFilterDropDownList)) { + if (currentFilter.startsWith(name)) + return QPlatformFileDialogHelper::cleanFilterList(currentFilter); } return QStringList(); }