Date: prev next · Thread: first prev next last
2012 Archives by date, by thread · List index


On 10/02/2012 07:40 PM, Libreoffice Gerrit user wrote:
commit 595ba03fd4769c069bbe0431ab878956ef1c37ad
Author: Michael Meeks <michael.meeks@suse.com>
Date:   Tue Oct 2 18:39:30 2012 +0100

     update string copy semantics to be undefined in a non-crashing way.

     Change-Id: I03bb4db5931932280e368012cbaee6bef2854dd6

diff --git a/sal/inc/rtl/string.hxx b/sal/inc/rtl/string.hxx
index 84ab48e..d58999b 100644
--- a/sal/inc/rtl/string.hxx
+++ b/sal/inc/rtl/string.hxx
@@ -1024,31 +1024,27 @@ public:
      /**
        Returns a new string that is a substring of this string.

-      The substring begins at the specified beginIndex.  It is an error for
-      beginIndex to be negative or to be greater than the length of this string.
+      The substring begins at the specified beginIndex. If
+      beginIndex is negative or be greater than the length of
+      this string, behaviour is undefined.

Given that "it is an error for X to happen" and "if X happens, behaviour is undefined" have exactly the same meaning (at least in my understanding of computing), I wonder whether this is just a harmless rephrasing, or whether there is a deeper misunderstanding lurking there.

diff --git a/sal/inc/rtl/ustring.hxx b/sal/inc/rtl/ustring.hxx
index b008054..e12a4b0 100644
--- a/sal/inc/rtl/ustring.hxx
+++ b/sal/inc/rtl/ustring.hxx
@@ -1372,17 +1368,9 @@ public:
      */
      OUString copy( sal_Int32 beginIndex, sal_Int32 count ) const SAL_THROW(())
      {
-        assert(beginIndex >= 0 && beginIndex <= getLength() && count >= 0
-               && sal::static_int_cast< sal_uInt32 >(count) <=
-                  sal::static_int_cast< sal_uInt32 >(getLength() - beginIndex));
-        if ( (beginIndex == 0) && (count == getLength()) )
-            return *this;
-        else
-        {
-            rtl_uString* pNew = 0;
-            rtl_uString_newFromStr_WithLength( &pNew, pData->buffer+beginIndex, count );
-            return OUString( pNew, (DO_NOT_ACQUIRE*)0 );
-        }
+        rtl_uString *pNew = 0;
+        rtl_uString_newFromSubString( &pNew, pData, beginIndex, count );
+        return OUString( pNew, (DO_NOT_ACQUIRE*)0 );
      }

      /**
diff --git a/sal/rtl/source/strtmpl.cxx b/sal/rtl/source/strtmpl.cxx
index ebb0da1..6069349 100644
--- a/sal/rtl/source/strtmpl.cxx
+++ b/sal/rtl/source/strtmpl.cxx
@@ -1194,6 +1194,29 @@ void SAL_CALL IMPL_RTL_STRINGNAME( newFromStr_WithLength )( 
IMPL_RTL_STRINGDATA*

  /* ----------------------------------------------------------------------- */

+void SAL_CALL IMPL_RTL_STRINGNAME( newFromSubString )( IMPL_RTL_STRINGDATA** ppThis,
+                                                       const IMPL_RTL_STRINGDATA* pFrom,
+                                                       sal_Int32 beginIndex,
+                                                       sal_Int32 count )
+    SAL_THROW_EXTERN_C()
+{
+    if ( beginIndex == 0 && count == pFrom->length )
+    {
+        IMPL_RTL_STRINGNAME( assign )( ppThis, const_cast< IMPL_RTL_STRINGDATA * >( pFrom ) );
+        return;
+    }
+    if ( count < 0 || beginIndex < 0 || beginIndex + count > pFrom->length )

Note how the original code above prevented problems with overflowing beginIndex + count.

+    {
+        OSL_FAIL( "Out of bounds substring access" );

New code should use SAL_WARN, please. (Arguably, it should rather use abort. Which would also make the following code moot.)

+        IMPL_RTL_STRINGNAME( newFromLiteral )( ppThis, "!!br0ken!!", 10, 0 );
+        return;
+    }
+
+    IMPL_RTL_STRINGNAME( newFromStr_WithLength )( ppThis, pFrom->buffer + beginIndex, count );
+}
+
+/* ----------------------------------------------------------------------- */
+
  // Used when creating from string literals.
  void SAL_CALL IMPL_RTL_STRINGNAME( newFromLiteral )( IMPL_RTL_STRINGDATA** ppThis,
                                                       const sal_Char* pCharStr,
diff --git a/sal/util/sal.map b/sal/util/sal.map
index 7a431c3..9efed6a 100644
--- a/sal/util/sal.map
+++ b/sal/util/sal.map
@@ -627,6 +627,12 @@ LIBO_UDK_3.6 { # symbols available in >= LibO 3.6
        rtl_uStringBuffer_makeStringAndClear;
  } UDK_3.10;

+LIBO_UDK_3.7 { # symbols available in >= LibO 3.7
+    global:
+        rtl_string_newFromSubString;
+        rtl_uString_newFromSubString;
+} UDK_3.10;

Oh, what should have been a linear list of versions turned into a multi-pronged fork starting with LIBO_UDK_3.5. Fixed that now for LIBO_UDK_3.7 at least.

Stephan

Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.