[Libreoffice-commits] core.git: connectivity/source offapi/com
Mike Kaganski (via logerrit)
logerrit at kemper.freedesktop.org
Fri Sep 10 16:54:32 UTC 2021
connectivity/source/drivers/firebird/Clob.cxx | 60 ++++++--------
connectivity/source/drivers/firebird/DatabaseMetaData.cxx | 2
connectivity/source/drivers/firebird/ResultSet.cxx | 2
offapi/com/sun/star/sdbc/XClob.idl | 2
4 files changed, 31 insertions(+), 35 deletions(-)
New commits:
commit ccea340276609c87384ee7fff4837e48baaba9b5
Author: Mike Kaganski <mike.kaganski at collabora.com>
AuthorDate: Fri Sep 10 16:12:10 2021 +0200
Commit: Mike Kaganski <mike.kaganski at collabora.com>
CommitDate: Fri Sep 10 18:53:57 2021 +0200
Clarify that css::sdbc::XClob::getSubString takes 1-based pos
... and fix connectivity::firebird::Clob::getSubString.
XClob::getSubString is modelled after JDBC counterpart, that also
takes 1-based position argument. This is also described in multiple
places in our code:
- OPreparedStatement::setClob
in connectivity/source/drivers/firebird/PreparedStatement.cxx
- OSingleSelectQueryComposer::setConditionByColumn
in dbaccess/source/core/api/SingleSelectQueryComposer.cxx
- ORowSetValue::getString
in connectivity/source/commontools/FValue.cxx
However in some places 0-based value was used (fixed here).
To clarify, the mention that the pos argument is 1-based is put
to the corresponding IDL file.
Also the code in connectivity::firebird::Clob::getSubString had
multiple issues:
- no checks of arguments;
- possibility to throw "nPosition out of range" when just-read
segment has enough data;
- wrong start position in case when nPosition is not aligned to
segment boundary.
This change fixes these, and simplifies the implementation.
Change-Id: I119a62dd7f02c9569ce36395ed8cc1a98c931fcf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121884
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>
diff --git a/connectivity/source/drivers/firebird/Clob.cxx b/connectivity/source/drivers/firebird/Clob.cxx
index 5825a5f2c871..707020b39bbf 100644
--- a/connectivity/source/drivers/firebird/Clob.cxx
+++ b/connectivity/source/drivers/firebird/Clob.cxx
@@ -71,55 +71,51 @@ sal_Int64 SAL_CALL Clob::length()
OUString SAL_CALL Clob::getSubString(sal_Int64 nPosition,
sal_Int32 nLength)
{
+ if (nPosition < 1) // XClob is indexed from 1
+ throw lang::IllegalArgumentException("nPosition < 1", *this, 0);
+ --nPosition; // make 0-based
+
+ if (nLength < 0)
+ throw lang::IllegalArgumentException("nLength < 0", *this, 0);
+
MutexGuard aGuard(m_aMutex);
checkDisposed(Clob_BASE::rBHelper.bDisposed);
// TODO do not reset position if it is not necessary
m_aBlob->closeInput(); // reset position
OUStringBuffer sSegmentBuffer;
- sal_Int64 nActPos = 1;
- sal_Int32 nActLen = 0;
std::vector<char> aSegmentBytes;
- // skip irrelevant parts
- while( nActPos < nPosition )
+ for (;;)
{
bool bLastRead = m_aBlob->readOneSegment( aSegmentBytes );
- if( bLastRead )
- throw lang::IllegalArgumentException("nPosition out of range", *this, 0);
-
+ // TODO: handle possible case of split UTF-8 character
OUString sSegment(aSegmentBytes.data(), aSegmentBytes.size(), RTL_TEXTENCODING_UTF8);
- sal_Int32 nStrLen = sSegment.getLength();
- nActPos += nStrLen;
- if( nActPos > nPosition )
+
+ // skip irrelevant parts
+ if (sSegment.getLength() < nPosition)
{
- sal_Int32 nCharsToCopy = static_cast<sal_Int32>(nActPos - nPosition);
- if( nCharsToCopy > nLength )
- nCharsToCopy = nLength;
- // append relevant part of first segment
- sSegmentBuffer.append( sSegment.subView(0, nCharsToCopy) );
- nActLen += sSegmentBuffer.getLength();
+ if (bLastRead)
+ throw lang::IllegalArgumentException("nPosition out of range", *this, 0);
+ nPosition -= sSegment.getLength();
+ continue;
}
- }
- // read nLength characters
- while( nActLen < nLength )
- {
- bool bLastRead = m_aBlob->readOneSegment( aSegmentBytes );
+ // Getting here for the first time, nPosition may be > 0, meaning copy start offset.
+ // This also handles sSegment.getLength() == nPosition case, including nLength == 0.
+ const sal_Int32 nCharsToCopy = std::min<sal_Int32>(sSegment.getLength() - nPosition,
+ nLength - sSegmentBuffer.getLength());
+ sSegmentBuffer.append(sSegment.subView(nPosition, nCharsToCopy));
+ if (sSegmentBuffer.getLength() == nLength)
+ return sSegmentBuffer.makeStringAndClear();
- OUString sSegment(aSegmentBytes.data(), aSegmentBytes.size(), RTL_TEXTENCODING_UTF8);
- sal_Int32 nStrLen = sSegment.getLength();
- if( nActLen + nStrLen > nLength )
- sSegmentBuffer.append(sSegment.subView(0, nLength - nActLen));
- else
- sSegmentBuffer.append(sSegment);
- nActLen += nStrLen;
-
- if( bLastRead && nActLen < nLength )
+ assert(sSegmentBuffer.getLength() < nLength);
+
+ if (bLastRead)
throw lang::IllegalArgumentException("out of range", *this, 0);
- }
- return sSegmentBuffer.makeStringAndClear();
+ nPosition = 0; // No offset after first append
+ }
}
uno::Reference< XInputStream > SAL_CALL Clob::getCharacterStream()
diff --git a/connectivity/source/drivers/firebird/DatabaseMetaData.cxx b/connectivity/source/drivers/firebird/DatabaseMetaData.cxx
index fde8809ec867..eff8097e25bd 100644
--- a/connectivity/source/drivers/firebird/DatabaseMetaData.cxx
+++ b/connectivity/source/drivers/firebird/DatabaseMetaData.cxx
@@ -1417,7 +1417,7 @@ uno::Reference< XResultSet > SAL_CALL ODatabaseMetaData::getTables(
uno::Reference< XClob > xClob = xRow->getClob(4);
if (xClob.is())
{
- aCurrentRow[5] = new ORowSetValueDecorator(xClob->getSubString(0, xClob->length()));
+ aCurrentRow[5] = new ORowSetValueDecorator(xClob->getSubString(1, xClob->length()));
}
}
diff --git a/connectivity/source/drivers/firebird/ResultSet.cxx b/connectivity/source/drivers/firebird/ResultSet.cxx
index 17e87cf8a55d..892e510138a3 100644
--- a/connectivity/source/drivers/firebird/ResultSet.cxx
+++ b/connectivity/source/drivers/firebird/ResultSet.cxx
@@ -611,7 +611,7 @@ OUString OResultSet::retrieveValue(const sal_Int32 nColumnIndex, const ISC_SHORT
else if(aSqlType == SQL_BLOB && aSqlSubType == static_cast<short>(BlobSubtype::Clob) )
{
uno::Reference<XClob> xClob = getClob(nColumnIndex);
- return xClob->getSubString( 0, xClob->length() );
+ return xClob->getSubString( 1, xClob->length() );
}
else
{
diff --git a/offapi/com/sun/star/sdbc/XClob.idl b/offapi/com/sun/star/sdbc/XClob.idl
index bc0d2bc81f0c..045dfc981768 100644
--- a/offapi/com/sun/star/sdbc/XClob.idl
+++ b/offapi/com/sun/star/sdbc/XClob.idl
@@ -117,7 +117,7 @@ published interface XClob: com::sun::star::uno::XInterface
consecutive characters.
</p>
@param pos
- the starting position
+ the starting position, 1-based
@param length
the length of the substring
@returns
More information about the Libreoffice-commits
mailing list