Changeset 24656 in vbox for trunk/src/VBox/Runtime

Timestamp:

Nov 14, 2009 10:36:32 PM (15 years ago)

Author:

vboxsync

Message:

iprt/ministring: bird review - addressed object state after throwing std::bad_alloc.
Because of the cleanup() + copyFrom() approach to changing the string, there is
no way to preserve the original string value without rewriting the code
fundamentally. I would strongly recommend doing so. (The rewrite is to not
cleanup() first, but use RTMemRealloc() to extend the buffer.)

Left a few review @todos in the startsWith, endsWith & contains
implementations. They are related and all depends on a policy decision wrt
matching empty strings.

File:

• trunk/src/VBox/Runtime/common/string/ministring.cpp (modified) (9 diffs)

trunk/src/VBox/Runtime/common/string/ministring.cpp

@@ -36 +36 @@
 using namespace iprt;
 
-const size_t MiniString::npos = (size_t)-1;
-
-/**
- * Appends a copy of @a that to "this".
- * @param that
- */
-MiniString& MiniString::append(const MiniString &that)
+const size_t MiniString::npos = ~(size_t)0;
+
+MiniString &MiniString::append(const MiniString &that)
 {
     size_t lenThat = that.length();
@@ -51 +47 @@
 
         reserve(cbBoth);
-            // calls realloc(cbBoth) and sets m_cbAllocated
+            // calls realloc(cbBoth) and sets m_cbAllocated; may throw bad_alloc.
+#ifndef RT_EXCEPTIONS_ENABLED
+        AssertRelease(capacity() >= cbBoth);
+#endif
 
         memcpy(m_psz + lenThis, that.m_psz, lenThat);
@@ -60 +59 @@
 }
 
-/**
- * Appends the given character to "this".
- * @param c
- * @return
- */
 MiniString& MiniString::append(char c)
 {
@@ -71 +65 @@
         // allocate in chunks of 20 in case this gets called several times
         if (m_cbLength + 1 >= m_cbAllocated)
+        {
             reserve(m_cbLength + 10);
-            // calls realloc() and sets m_cbAllocated
+            // calls realloc(cbBoth) and sets m_cbAllocated; may throw bad_alloc.
+#ifndef RT_EXCEPTIONS_ENABLED
+            AssertRelease(capacity() >= m_cbLength + 1);
+#endif
+        }
 
         m_psz[m_cbLength] = c;
@@ -81 +80 @@
 }
 
-size_t MiniString::find(const char *pcszFind,
-                        size_t pos /*= 0*/)
+size_t MiniString::find(const char *pcszFind, size_t pos /*= 0*/)
     const
 {
@@ -128 +126 @@
 
                 size_t cbCopy = psz - pFirst;
-                ret.reserve(cbCopy + 1);
+                ret.reserve(cbCopy + 1); // may throw bad_alloc
+#ifndef RT_EXCEPTIONS_ENABLED
+                AssertRelease(capacity() >= cbCopy + 1);
+#endif
                 memcpy(ret.m_psz, pFirst, cbCopy);
                 ret.m_cbLength = cbCopy;
@@ -148 +149 @@
     if (l1 < l2)
         return false;
+    /** @todo r=bird: If l2 is 0, then m_psz can be NULL and we will crash. See
+     *        also handling of l2 == in startsWith. */
 
     size_t l = l1 - l2;
@@ -160 +163 @@
     size_t l1 = length();
     size_t l2 = that.length();
-    if (l1 == 0 || l2 == 0)
+    if (l1 == 0 || l2 == 0) /** @todo r=bird: this differs from endsWith, and I think other IPRT code. If l2 == 0, it matches anything. */
         return false;
 
@@ -174 +177 @@
 bool MiniString::contains(const MiniString &that, CaseSensitivity cs /*= CaseSensitive*/) const
 {
+    /** @todo r-bird: Not checking for NULL strings like startsWith does (and
+     *        endsWith only does half way). */
     if (cs == CaseSensitive)
         return ::RTStrStr(m_psz, that.m_psz) != NULL;