New loplugin:unsignedcompare

"Find explicit casts from signed to unsigned integer in comparison against
unsigned integer, where the cast is presumably used to avoid warnings about
signed vs. unsigned comparisons, and could thus be replaced with
o3tl::make_unsigned for clairty." (compilerplugins/clang/unsignedcompare.cxx)

o3tl::make_unsigned requires its argument to be non-negative, and there is a
chance that some original code like

static_cast<sal_uInt32>(n) >= c

used the explicit cast to actually force a (potentially negative) value of
sal_Int32 to be interpreted as an unsigned sal_uInt32, rather than using the
cast to avoid a false "signed vs. unsigned comparison" warning in a case where
n is known to be non-negative.  It appears that restricting this plugin to non-
equality comparisons (<, >, <=, >=) and excluding equality comparisons (==, !=)
is a useful heuristic to avoid such false positives.  The only remainging false
positive I found was 0288c8ffecff4956a52b9147d441979941e8b87f "Rephrase cast
from sal_Int32 to sal_uInt32".

But which of course does not mean that there were no further false positivies
that I missed.  So this commit may accidentally introduce some false hits of the
assert in o3tl::make_unsigned.  At least, it passed a full (Linux ASan+UBSan
--enable-dbgutil) `make check && make screenshot`.

It is by design that o3tl::make_unsigned only accepts signed integer parameter
types (and is not defined as a nop for unsigned ones), to avoid unnecessary uses
which would in general be suspicious.  But the STATIC_ARRAY_SELECT macro in
include/oox/helper/helper.hxx is used with both signed and unsigned types, so
needs a little oox::detail::make_unsigned helper function for now.  (The
ultimate fix being to get rid of the macro in the first place.)

Change-Id: Ia4adc9f44c70ad1dfd608784cac39ee922c32175
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/87556
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

10 files changed, 30 insertions, 21 deletions

diff --git a/dbaccess/source/core/api/RowSet.cxx b/dbaccess/source/core/api/RowSet.cxx
index 10432ffbfb7b..7e80d5e9ec22 100644
--- a/dbaccess/source/core/api/RowSet.cxx
+++ b/dbaccess/source/core/api/RowSet.cxx
@@ -71,6 +71,7 @@
 #include <cppuhelper/supportsservice.hxx>
 #include <cppuhelper/typeprovider.hxx>
 #include <i18nlangtag/languagetag.hxx>
+#include <o3tl/safeint.hxx>
 #include <unotools/syslocale.hxx>
 #include <tools/debug.hxx>
 #include <tools/diagnose_ex.h>
@@ -1455,7 +1456,7 @@ void SAL_CALL ORowSet::executeWithCompletion( const Reference< XInteractionHandl
 
         Reference<XIndexAccess>  xParamsAsIndicies = xParameters.is() ? xParameters->getParameters() : Reference<XIndexAccess>();
         const sal_Int32 nParamCount = xParamsAsIndicies.is() ? xParamsAsIndicies->getCount() : 0;
-        if ( m_aParametersSet.size() < static_cast<size_t>(nParamCount) )
+        if ( m_aParametersSet.size() < o3tl::make_unsigned(nParamCount) )
             m_aParametersSet.resize( nParamCount ,false);
 
         ::dbtools::askForParameters( xComposer, this, m_xActiveConnection, _rxHandler,m_aParametersSet );
@@ -2438,7 +2439,7 @@ ORowSetValue& ORowSet::getParameterStorage(sal_Int32 parameterIndex)
     if ( parameterIndex < 1 )
         throwInvalidIndexException( *this );
 
-    if ( m_aParametersSet.size() < static_cast<size_t>(parameterIndex) )
+    if ( m_aParametersSet.size() < o3tl::make_unsigned(parameterIndex) )
         m_aParametersSet.resize( parameterIndex ,false);
     m_aParametersSet[parameterIndex - 1] = true;
 
@@ -2450,13 +2451,13 @@ ORowSetValue& ORowSet::getParameterStorage(sal_Int32 parameterIndex)
             impl_disposeParametersContainer_nothrow();
         if ( m_pParameters.is() )
         {
-            if ( static_cast<size_t>(parameterIndex) > m_pParameters->size() )
+            if ( o3tl::make_unsigned(parameterIndex) > m_pParameters->size() )
                 throwInvalidIndexException( *this );
             return (*m_pParameters)[ parameterIndex - 1 ];
         }
     }
 
-    if ( m_aPrematureParamValues->get().size() < static_cast<size_t>(parameterIndex) )
+    if ( m_aPrematureParamValues->get().size() < o3tl::make_unsigned(parameterIndex) )
         m_aPrematureParamValues->get().resize( parameterIndex );
     return m_aPrematureParamValues->get()[ parameterIndex - 1 ];
 }
diff --git a/dbaccess/source/core/api/RowSetBase.cxx b/dbaccess/source/core/api/RowSetBase.cxx
index c203056815cd..81ba20a3b7c1 100644
--- a/dbaccess/source/core/api/RowSetBase.cxx
+++ b/dbaccess/source/core/api/RowSetBase.cxx
@@ -35,6 +35,7 @@
 #include <comphelper/sequence.hxx>
 #include <comphelper/seqstream.hxx>
 #include <connectivity/dbexception.hxx>
+#include <o3tl/safeint.hxx>
 #include <tools/debug.hxx>
 
 using namespace dbaccess;
@@ -233,7 +234,7 @@ const ORowSetValue& ORowSetBase::impl_getValue(sal_Int32 columnIndex)
         }
         OSL_ENSURE(!m_aCurrentRow.isNull() && m_aCurrentRow < m_pCache->getEnd() && aCacheIter != m_pCache->m_aCacheIterators.end(),"Invalid iterator set for currentrow!");
         ORowSetRow rRow = *m_aCurrentRow;
-        OSL_ENSURE(rRow.is() && static_cast<sal_uInt16>(columnIndex) < (rRow->get()).size(),"Invalid size of vector!");
+        OSL_ENSURE(rRow.is() && o3tl::make_unsigned(columnIndex) < (rRow->get()).size(),"Invalid size of vector!");
 #endif
         return ((*m_aCurrentRow)->get())[m_nLastColumnIndex = columnIndex];
     }
diff --git a/dbaccess/source/core/api/RowSetCache.cxx b/dbaccess/source/core/api/RowSetCache.cxx
index 1ca8ba61b7da..f5ca34c20667 100644
--- a/dbaccess/source/core/api/RowSetCache.cxx
+++ b/dbaccess/source/core/api/RowSetCache.cxx
@@ -52,6 +52,7 @@
 #include <connectivity/sqlparse.hxx>
 #include <sqlbison.hxx>
 #include <tools/diagnose_ex.h>
+#include <o3tl/safeint.hxx>
 #include <osl/diagnose.h>
 
 #include <algorithm>
@@ -880,7 +881,7 @@ void ORowSetCache::moveWindow()
             const sal_Int32 nOverlapSize = m_nEndPos - m_nStartPos;
             const sal_Int32 nStartPosOffset = m_nStartPos - nNewStartPos; // by how much m_nStartPos moves
             m_nStartPos = nNewStartPos;
-            OSL_ENSURE( static_cast<ORowSetMatrix::size_type>(nOverlapSize) <= m_pMatrix->size(), "new window end is after end of cache matrix!" );
+            OSL_ENSURE( o3tl::make_unsigned(nOverlapSize) <= m_pMatrix->size(), "new window end is after end of cache matrix!" );
             // the first position in m_pMatrix whose data we don't keep;
             // content will be moved to m_pMatrix.begin()
             ORowSetMatrix::iterator aEnd (m_pMatrix->begin() + nOverlapSize);
diff --git a/dbaccess/source/ui/browser/formadapter.cxx b/dbaccess/source/ui/browser/formadapter.cxx
index aea58079ff23..cd1f0e958f14 100644
--- a/dbaccess/source/ui/browser/formadapter.cxx
+++ b/dbaccess/source/ui/browser/formadapter.cxx
@@ -18,6 +18,7 @@
  */
 
 #include <formadapter.hxx>
+#include <o3tl/safeint.hxx>
 #include <osl/diagnose.h>
 #include <comphelper/types.hxx>
 #include <comphelper/enumhelper.hxx>
@@ -1491,14 +1492,14 @@ sal_Bool SAL_CALL SbaXFormAdapter::hasElements()
 // css::container::XIndexContainer
 void SAL_CALL SbaXFormAdapter::insertByIndex(sal_Int32 _rIndex, const Any& Element)
 {
-    if ( ( _rIndex < 0 ) || ( static_cast<size_t>(_rIndex) >= m_aChildren.size() ) )
+    if ( ( _rIndex < 0 ) || ( o3tl::make_unsigned(_rIndex) >= m_aChildren.size() ) )
         throw css::lang::IndexOutOfBoundsException();
     implInsert(Element, _rIndex);
 }
 
 void SAL_CALL SbaXFormAdapter::removeByIndex(sal_Int32 _rIndex)
 {
-    if ( ( _rIndex < 0 ) || ( static_cast<size_t>(_rIndex) >= m_aChildren.size() ) )
+    if ( ( _rIndex < 0 ) || ( o3tl::make_unsigned(_rIndex) >= m_aChildren.size() ) )
         throw css::lang::IndexOutOfBoundsException();
 
     Reference< css::form::XFormComponent >  xAffected = *(m_aChildren.begin() + _rIndex);
@@ -1527,7 +1528,7 @@ void SAL_CALL SbaXFormAdapter::removeByIndex(sal_Int32 _rIndex)
 // css::container::XIndexReplace
 void SAL_CALL SbaXFormAdapter::replaceByIndex(sal_Int32 _rIndex, const Any& Element)
 {
-    if ( ( _rIndex < 0 ) || ( static_cast<size_t>(_rIndex) >= m_aChildren.size() ) )
+    if ( ( _rIndex < 0 ) || ( o3tl::make_unsigned(_rIndex) >= m_aChildren.size() ) )
         throw css::lang::IndexOutOfBoundsException();
 
     // extract the form component
@@ -1594,7 +1595,7 @@ sal_Int32 SAL_CALL SbaXFormAdapter::getCount()
 
 Any SAL_CALL SbaXFormAdapter::getByIndex(sal_Int32 _rIndex)
 {
-    if ( ( _rIndex < 0 ) || ( static_cast<size_t>(_rIndex) >= m_aChildren.size() ) )
+    if ( ( _rIndex < 0 ) || ( o3tl::make_unsigned(_rIndex) >= m_aChildren.size() ) )
         throw css::lang::IndexOutOfBoundsException();
 
     Reference< css::form::XFormComponent >  xElement = *(m_aChildren.begin() + _rIndex);
diff --git a/dbaccess/source/ui/control/RelationControl.cxx b/dbaccess/source/ui/control/RelationControl.cxx
index 004096c45fa4..2f1421d85e7e 100644
--- a/dbaccess/source/ui/control/RelationControl.cxx
+++ b/dbaccess/source/ui/control/RelationControl.cxx
@@ -33,6 +33,7 @@
 #include <UITools.hxx>
 #include <RelControliFace.hxx>
 #include <helpids.h>
+#include <o3tl/safeint.hxx>
 #include <osl/diagnose.h>
 
 #include <vector>
@@ -221,7 +222,7 @@ namespace dbaui
         {
             OUString sFieldName(m_pListCell->GetSelectedEntry());
             OConnectionLineDataVec& rLines = m_pConnData->GetConnLineDataList();
-            if ( rLines.size() <= static_cast<OConnectionLineDataVec::size_type>(nRow) )
+            if ( rLines.size() <= o3tl::make_unsigned(nRow) )
             {
                 rLines.push_back(new OConnectionLineData());
                 nRow = rLines.size() - 1;
@@ -266,7 +267,7 @@ namespace dbaui
     OUString ORelationControl::GetCellText( long nRow, sal_uInt16 nColId ) const
     {
         OUString sText;
-        if ( m_pConnData->GetConnLineDataList().size() > static_cast<size_t>(nRow) )
+        if ( m_pConnData->GetConnLineDataList().size() > o3tl::make_unsigned(nRow) )
         {
             OConnectionLineDataRef pConnLineData = m_pConnData->GetConnLineDataList()[nRow];
             switch( getColumnIdent( nColId ) )
diff --git a/dbaccess/source/ui/dlg/generalpage.cxx b/dbaccess/source/ui/dlg/generalpage.cxx
index cf564821fcf0..b94b63e9f24a 100644
--- a/dbaccess/source/ui/dlg/generalpage.cxx
+++ b/dbaccess/source/ui/dlg/generalpage.cxx
@@ -40,6 +40,7 @@
 #include <UITools.hxx>
 #include <comphelper/processfactory.hxx>
 #include <unotools/confignode.hxx>
+#include <o3tl/safeint.hxx>
 #include <osl/diagnose.h>
 #include <sal/log.hxx>
 #include <dbwizsetup.hxx>
@@ -366,7 +367,7 @@ namespace dbaui
     {
         // get the type from the entry data
         const sal_Int32 nSelected = _rBox.get_active();
-        if (static_cast<size_t>(nSelected) >= m_aEmbeddedURLPrefixes.size() )
+        if (o3tl::make_unsigned(nSelected) >= m_aEmbeddedURLPrefixes.size() )
         {
             SAL_WARN("dbaccess.ui.generalpage", "Got out-of-range value '" << nSelected <<  "' from the DatasourceType selection ListBox's GetSelectedEntryPos(): no corresponding URL prefix");
             return;
@@ -386,7 +387,7 @@ namespace dbaui
         const sal_Int32 nSelected = _rBox.get_active();
         if (nSelected == -1)
             return;
-        if (static_cast<size_t>(nSelected) >= m_aURLPrefixes.size() )
+        if (o3tl::make_unsigned(nSelected) >= m_aURLPrefixes.size() )
         {
             SAL_WARN("dbaccess.ui.generalpage", "Got out-of-range value '" << nSelected <<  "' from the DatasourceType selection ListBox's GetSelectedEntryPos(): no corresponding URL prefix");
             return;
diff --git a/dbaccess/source/ui/dlg/paramdialog.cxx b/dbaccess/source/ui/dlg/paramdialog.cxx
index 982e28bba49e..dc7d01dfcbc8 100644
--- a/dbaccess/source/ui/dlg/paramdialog.cxx
+++ b/dbaccess/source/ui/dlg/paramdialog.cxx
@@ -30,6 +30,7 @@
 #include <stringconstants.hxx>
 #include <vcl/svapp.hxx>
 #include <vcl/weld.hxx>
+#include <o3tl/safeint.hxx>
 #include <osl/diagnose.h>
 #include <tools/diagnose_ex.h>
 #include <unotools/syslocale.hxx>
@@ -295,7 +296,7 @@ namespace dbaui
         m_nCurrentlySelected = nSelected;
 
         // with this the value isn't dirty
-        OSL_ENSURE(static_cast<size_t>(m_nCurrentlySelected) < m_aVisitedParams.size(), "OParameterDialog::OnEntrySelected : invalid current entry !");
+        OSL_ENSURE(o3tl::make_unsigned(m_nCurrentlySelected) < m_aVisitedParams.size(), "OParameterDialog::OnEntrySelected : invalid current entry !");
         m_aVisitedParams[m_nCurrentlySelected] &= ~VisitFlags::Dirty;
 
         m_aResetVisitFlag.SetTimeout(1000);
@@ -309,7 +310,7 @@ namespace dbaui
         OSL_ENSURE(m_nCurrentlySelected != -1, "OParameterDialog::OnVisitedTimeout : invalid call !");
 
         // mark the currently selected entry as visited
-        OSL_ENSURE(static_cast<size_t>(m_nCurrentlySelected) < m_aVisitedParams.size(), "OParameterDialog::OnVisitedTimeout : invalid entry !");
+        OSL_ENSURE(o3tl::make_unsigned(m_nCurrentlySelected) < m_aVisitedParams.size(), "OParameterDialog::OnVisitedTimeout : invalid entry !");
         m_aVisitedParams[m_nCurrentlySelected] |= VisitFlags::Visited;
 
         // was it the last "not visited yet" entry ?
@@ -334,7 +335,7 @@ namespace dbaui
     IMPL_LINK(OParameterDialog, OnValueModified, weld::Entry&, rEdit, void)
     {
         // mark the currently selected entry as dirty
-        OSL_ENSURE(static_cast<size_t>(m_nCurrentlySelected) < m_aVisitedParams.size(), "OParameterDialog::OnValueModified : invalid entry !");
+        OSL_ENSURE(o3tl::make_unsigned(m_nCurrentlySelected) < m_aVisitedParams.size(), "OParameterDialog::OnValueModified : invalid entry !");
         m_aVisitedParams[m_nCurrentlySelected] |= VisitFlags::Dirty;
         rEdit.set_message_type(weld::EntryMessageType::Normal);
     }
diff --git a/dbaccess/source/ui/misc/WCopyTable.cxx b/dbaccess/source/ui/misc/WCopyTable.cxx
index 6be2d87fa9a9..ca85547679ce 100644
--- a/dbaccess/source/ui/misc/WCopyTable.cxx
+++ b/dbaccess/source/ui/misc/WCopyTable.cxx
@@ -52,7 +52,7 @@
 #include <connectivity/dbtools.hxx>
 #include <connectivity/dbmetadata.hxx>
 #include <connectivity/dbexception.hxx>
-
+#include <o3tl/safeint.hxx>
 #include <rtl/ustrbuf.hxx>
 #include <sal/log.hxx>
 #include <tools/debug.hxx>
@@ -1258,7 +1258,7 @@ Reference< XPropertySet > OCopyTableWizard::createTable()
                     if ( m_vColumnPositions.end() != aPosFind )
                     {
                         aPosFind->second = nNewPos;
-                        OSL_ENSURE( m_vColumnTypes.size() > size_t( aPosFind - m_vColumnPositions.begin() ),
+                        OSL_ENSURE( m_vColumnTypes.size() > o3tl::make_unsigned( aPosFind - m_vColumnPositions.begin() ),
                             "Invalid index for vector!" );
                         m_vColumnTypes[ aPosFind - m_vColumnPositions.begin() ] = (*aFind)->second->GetType();
                     }
diff --git a/dbaccess/source/ui/querydesign/JAccess.cxx b/dbaccess/source/ui/querydesign/JAccess.cxx
index f2459ee67d32..703d91fe30f9 100644
--- a/dbaccess/source/ui/querydesign/JAccess.cxx
+++ b/dbaccess/source/ui/querydesign/JAccess.cxx
@@ -25,6 +25,7 @@
 #include <JoinDesignView.hxx>
 #include <JoinController.hxx>
 #include <TableConnection.hxx>
+#include <o3tl/safeint.hxx>
 
 namespace dbaui
 {
@@ -71,7 +72,7 @@ namespace dbaui
             OJoinTableView::OTableWindowMap::const_iterator aIter = std::next(m_pTableView->GetTabWinMap().begin(), i);
             aRet = aIter->second->GetAccessible();
         }
-        else if( size_t(i - nTableWindowCount) < m_pTableView->getTableConnections().size() )
+        else if( o3tl::make_unsigned(i - nTableWindowCount) < m_pTableView->getTableConnections().size() )
             aRet = m_pTableView->getTableConnections()[i - nTableWindowCount]->GetAccessible();
         return aRet;
     }
diff --git a/dbaccess/source/ui/querydesign/QueryDesignView.cxx b/dbaccess/source/ui/querydesign/QueryDesignView.cxx
index 231d8fd6a701..da95ed304dae 100644
--- a/dbaccess/source/ui/querydesign/QueryDesignView.cxx
+++ b/dbaccess/source/ui/querydesign/QueryDesignView.cxx
@@ -26,6 +26,7 @@
 #include <vcl/split.hxx>
 #include <svl/undo.hxx>
 #include <tools/diagnose_ex.h>
+#include <o3tl/safeint.hxx>
 #include <osl/diagnose.h>
 #include <adtabdlg.hxx>
 #include <vcl/svapp.hxx>
@@ -2121,7 +2122,7 @@ namespace
                         sal_Int32 nFunctionType = FKT_NONE;
                         ::connectivity::OSQLParseNode* pParamRef = nullptr;
                         sal_Int32 nColumnRefPos = pColumnRef->count() - 2;
-                        if ( nColumnRefPos >= 0 && static_cast<sal_uInt32>(nColumnRefPos) < pColumnRef->count() )
+                        if ( nColumnRefPos >= 0 && o3tl::make_unsigned(nColumnRefPos) < pColumnRef->count() )
                             pParamRef = pColumnRef->getChild(nColumnRefPos);
 
                         if ( SQL_ISRULE(pColumnRef,general_set_fct)