desktop/source/lib/init.cxx | 6 +++---
include/rtl/string.hxx | 3 +++
include/rtl/ustring.hxx | 3 +++
include/sfx2/viewsh.hxx | 8 ++++----
sfx2/source/view/lokstarmathhelper.cxx | 4 ++--
sfx2/source/view/viewsh.cxx | 8 ++++----
test/source/lokcallback.cxx | 8 ++++----
7 files changed, 23 insertions(+), 17 deletions(-)
New commits:
commit fd156c9c2ff1b10e1f3d72c0001f36676e412e5b
Author: Caolán McNamara <caolan.mcnam...@collabora.com>
AuthorDate: Wed Jan 24 09:41:17 2024 +0000
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Wed Jan 24 14:32:56 2024 +0100
SfxViewShell::GetFirst lambda is never passed a null pShell
it's null checked before the lambda gets called so make this a
reference
Change-Id: Ib8804a2003cbdc6b7b62d8a38fa514d0ce08128c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162495
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index a2675fd97871..ebef6305c2ad 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -2280,7 +2280,7 @@ void CallbackFlushHandler::enqueueUpdatedTypes()
return;
assert(m_viewId >= 0);
SfxViewShell* viewShell = SfxViewShell::GetFirst( false,
- [this](const SfxViewShell* shell) { return
shell->GetViewShellId().get() == m_viewId; } );
+ [this](const SfxViewShell& shell) { return
shell.GetViewShellId().get() == m_viewId; } );
assert(viewShell != nullptr);
// First move data to local structures, so that callbacks don't possibly
modify it.
@@ -2323,7 +2323,7 @@ void CallbackFlushHandler::enqueueUpdatedTypes()
{
assert(sourceViewId >= 0);
sourceViewShell = SfxViewShell::GetFirst( false,
- [sourceViewId](const SfxViewShell* shell) { return
shell->GetViewShellId().get() == sourceViewId; } );
+ [sourceViewId](const SfxViewShell& shell) { return
shell.GetViewShellId().get() == sourceViewId; } );
}
if(sourceViewShell == nullptr)
{
@@ -2364,7 +2364,7 @@ void CallbackFlushHandler::Invoke()
// so it must be done before taking the mutex.
assert(m_viewId >= 0);
if(SfxViewShell* viewShell = SfxViewShell::GetFirst( false,
- [this](const SfxViewShell* shell) { return
shell->GetViewShellId().get() == m_viewId; } ))
+ [this](const SfxViewShell& shell) { return
shell.GetViewShellId().get() == m_viewId; } ))
{
viewShell->flushPendingLOKInvalidateTiles();
}
diff --git a/include/sfx2/viewsh.hxx b/include/sfx2/viewsh.hxx
index 0cbadc5072dc..805b7ee7893a 100644
--- a/include/sfx2/viewsh.hxx
+++ b/include/sfx2/viewsh.hxx
@@ -150,9 +150,9 @@ public: \
#define SFX_VIEW_REGISTRATION(DocClass) \
DocClass::Factory().RegisterViewFactory( *Factory() )
-template<class T> bool checkSfxViewShell(const SfxViewShell* pShell)
+template<class T> bool checkSfxViewShell(const SfxViewShell& pShell)
{
- return dynamic_cast<const T*>(pShell) != nullptr;
+ return dynamic_cast<const T*>(&pShell) != nullptr;
}
typedef std::unordered_map<OUString, std::pair<Color, int>>
StylesHighlighterColorMap;
@@ -204,10 +204,10 @@ protected:
public:
// Iteration
- SAL_WARN_UNUSED_RESULT static SfxViewShell* GetFirst( bool bOnlyVisible =
true, const std::function<bool ( const SfxViewShell* )>& isViewShell = nullptr
);
+ SAL_WARN_UNUSED_RESULT static SfxViewShell* GetFirst( bool bOnlyVisible =
true, const std::function<bool ( const SfxViewShell& )>& isViewShell = nullptr
);
SAL_WARN_UNUSED_RESULT static SfxViewShell* GetNext( const SfxViewShell&
rPrev,
bool bOnlyVisible = true,
- const std::function<bool ( const
SfxViewShell* )>& isViewShell = nullptr );
+ const std::function<bool ( const
SfxViewShell& )>& isViewShell = nullptr );
SAL_WARN_UNUSED_RESULT static SfxViewShell* Current();
SAL_WARN_UNUSED_RESULT static SfxViewShell* Get( const
css::uno::Reference< css::frame::XController>& i_rController );
diff --git a/sfx2/source/view/lokstarmathhelper.cxx
b/sfx2/source/view/lokstarmathhelper.cxx
index 9b2df19ecdec..e8c4b3a6ca7c 100644
--- a/sfx2/source/view/lokstarmathhelper.cxx
+++ b/sfx2/source/view/lokstarmathhelper.cxx
@@ -130,8 +130,8 @@ const SfxViewShell* LokStarMathHelper::GetSmViewShell()
{
if (vcl::Window* pGraphWindow = GetGraphicWindow())
{
- return SfxViewShell::GetFirst(false, [pGraphWindow](const
SfxViewShell* shell) {
- return shell->GetWindow() &&
shell->GetWindow()->IsChild(pGraphWindow);
+ return SfxViewShell::GetFirst(false, [pGraphWindow](const
SfxViewShell& shell) {
+ return shell.GetWindow() &&
shell.GetWindow()->IsChild(pGraphWindow);
});
}
return nullptr;
diff --git a/sfx2/source/view/viewsh.cxx b/sfx2/source/view/viewsh.cxx
index ca8db9d63a5d..06dfc49798be 100644
--- a/sfx2/source/view/viewsh.cxx
+++ b/sfx2/source/view/viewsh.cxx
@@ -3007,7 +3007,7 @@ void SfxViewShell::WriteUserDataSequence ( uno::Sequence
< beans::PropertyValue
SfxViewShell* SfxViewShell::GetFirst
(
bool bOnlyVisible,
- const std::function< bool ( const SfxViewShell* ) >& isViewShell
+ const std::function< bool ( const SfxViewShell& ) >& isViewShell
)
{
// search for a SfxViewShell of the specified type
@@ -3024,7 +3024,7 @@ SfxViewShell* SfxViewShell::GetFirst
// That doesn't seem to be needed anymore, but keep an assert,
just in case.
assert(std::find(SfxGetpApp()->GetViewFrames_Impl().begin(),
SfxGetpApp()->GetViewFrames_Impl().end(),
&pShell->GetViewFrame()) !=
SfxGetpApp()->GetViewFrames_Impl().end());
- if ( ( !bOnlyVisible || pShell->GetViewFrame().IsVisible() ) &&
(!isViewShell || isViewShell(pShell)))
+ if ( ( !bOnlyVisible || pShell->GetViewFrame().IsVisible() ) &&
(!isViewShell || isViewShell(*pShell)))
return pShell;
}
}
@@ -3037,7 +3037,7 @@ SfxViewShell* SfxViewShell::GetNext
(
const SfxViewShell& rPrev,
bool bOnlyVisible,
- const std::function<bool ( const SfxViewShell* )>& isViewShell
+ const std::function<bool ( const SfxViewShell& )>& isViewShell
)
{
std::vector<SfxViewShell*> &rShells = SfxGetpApp()->GetViewShells_Impl();
@@ -3053,7 +3053,7 @@ SfxViewShell* SfxViewShell::GetNext
{
assert(std::find(SfxGetpApp()->GetViewFrames_Impl().begin(),
SfxGetpApp()->GetViewFrames_Impl().end(),
&pShell->GetViewFrame()) !=
SfxGetpApp()->GetViewFrames_Impl().end());
- if ( ( !bOnlyVisible || pShell->GetViewFrame().IsVisible() ) &&
(!isViewShell || isViewShell(pShell)) )
+ if ( ( !bOnlyVisible || pShell->GetViewFrame().IsVisible() ) &&
(!isViewShell || isViewShell(*pShell)) )
return pShell;
}
}
diff --git a/test/source/lokcallback.cxx b/test/source/lokcallback.cxx
index a979b8b50582..0914ea4db73c 100644
--- a/test/source/lokcallback.cxx
+++ b/test/source/lokcallback.cxx
@@ -150,8 +150,8 @@ void TestLokCallbackWrapper::flushLOKData()
return;
// Ask for payloads of all the pending types that need updating, and call
the generic callback with that data.
assert(m_viewId >= 0);
- SfxViewShell* viewShell = SfxViewShell::GetFirst(false, [this](const
SfxViewShell* shell) {
- return shell->GetViewShellId().get() == m_viewId;
+ SfxViewShell* viewShell = SfxViewShell::GetFirst(false, [this](const
SfxViewShell& shell) {
+ return shell.GetViewShellId().get() == m_viewId;
});
assert(viewShell != nullptr);
// First move data to local structures, so that callbacks don't possibly
modify it.
@@ -168,8 +168,8 @@ void TestLokCallbackWrapper::flushLOKData()
}
for (const PerViewIdData& data : updatedTypesPerViewId)
{
- viewShell = SfxViewShell::GetFirst(false, [data](const SfxViewShell*
shell) {
- return shell->GetViewShellId().get() == data.sourceViewId;
+ viewShell = SfxViewShell::GetFirst(false, [data](const SfxViewShell&
shell) {
+ return shell.GetViewShellId().get() == data.sourceViewId;
});
assert(viewShell != nullptr);
std::optional<OString> payload = viewShell->getLOKPayload(data.type,
data.viewId);
commit 5950db59fc3f834425c342fff5a0fe9987d72144
Author: Caolán McNamara <caolan.mcnam...@collabora.com>
AuthorDate: Sun Jan 21 12:13:33 2024 +0000
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Wed Jan 24 14:32:41 2024 +0100
suppress COPY_INSTEAD_OF_MOVE suggestions for rtl::O[U]String
where we have implemented move assignment to take let the compiler take
advantage of any little optimization possibilities that it can take but
where the potential optimization probably doesn't outweigh enforcing
dusting error-prone std::move all over every case where the compiler
doesn't/can't use the move assignment but could.
so convert the myriad of micro optimization warnings into a single
missing move assignment warning which can be then suppressed.
Change-Id: I664193f9a2ac5014cf8d5134105ebd3a36857830
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162494
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
diff --git a/include/rtl/string.hxx b/include/rtl/string.hxx
index 93aca37ffd7d..d9a5bf3c3bac 100644
--- a/include/rtl/string.hxx
+++ b/include/rtl/string.hxx
@@ -188,6 +188,7 @@ template<OStringLiteral L> struct OStringHolder {
use this class.
*/
+// coverity[ missing_move_assignment : SUPPRESS] - don't report the suppressed
move assignment
class SAL_WARN_UNUSED SAL_DLLPUBLIC_RTTI OString
{
public:
@@ -509,6 +510,7 @@ public:
}
#if defined LIBO_INTERNAL_ONLY
+#if !defined(__COVERITY__) // suppress COPY_INSTEAD_OF_MOVE suggestions
/**
Move assign a new string.
@@ -523,6 +525,7 @@ public:
rtl_string_new( &str.pData );
return *this;
}
+#endif
#endif
/**
diff --git a/include/rtl/ustring.hxx b/include/rtl/ustring.hxx
index 9d897f2e84de..14a9d2cf8b36 100644
--- a/include/rtl/ustring.hxx
+++ b/include/rtl/ustring.hxx
@@ -167,6 +167,7 @@ template<std::size_t N> struct
ExceptCharArrayDetector<OUStringLiteral<N>> {};
less people should have understanding problems when they use this class.
*/
+// coverity[ missing_move_assignment : SUPPRESS] - don't report the suppressed
move assignment
class SAL_WARN_UNUSED SAL_DLLPUBLIC_RTTI OUString
{
public:
@@ -587,6 +588,7 @@ public:
}
#if defined LIBO_INTERNAL_ONLY
+#if !defined(__COVERITY__) // suppress COPY_INSTEAD_OF_MOVE suggestions
/**
Move assign a new string.
@@ -598,6 +600,7 @@ public:
std::swap(pData, str.pData);
return *this;
}
+#endif
#endif
/**
