Changeset 11684 in webkit for trunk/JavaScriptCore

Timestamp:

Dec 20, 2005, 12:12:50 PM (19 years ago)

Author:

mjs

Message:

JavaScriptCore:

Reviewed by Darin.

• fixed a leak in the assignment operator from PassRefPtr to RefPtr

• kxmlcore/RefPtr.h: (KXMLCore::RefPtr::operator=):

• fix problem with PassRefPtr that darin spotted - it lacked a copy constructor and therefore was using the default one, which can lead to excess derefs

I fixed this by adding a copy constructor from non-const
reference, and by adding a template pass() function that you have
to use when raw pointer or RefPtr are passed where PassRefPtr is
expected.

• kjs/identifier.cpp: (KJS::Identifier::add): Changed to have PassRefPtr return type and pass() the results.
• kjs/identifier.h:
• kjs/property_map.cpp: (KJS::PropertyMap::addSparseArrayPropertiesToReferenceList): Use pass() where required.
• kjs/ustring.cpp: (KJS::UString::UString): Use pass() as needed. (KJS::UString::append): ditto (KJS::UString::substr): ditto
• kjs/ustring.h: (KJS::UString::UString): Use initializer instead of assignment
• kxmlcore/PassRefPtr.h: (KXMLCore::PassRefPtr::PassRefPtr): Added copy constructor (KXMLCore::pass): new template function to make it convenient to pass a PassRefPtr

WebCore:

Reviewed by Darin.

• change an assignment to a contructor declaration to build with PassRefPtr leak fix changes

• ksvg2/svg/SVGTransformableImpl.cpp: (SVGTransformableImpl::parseTransformAttribute):

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• kjs/identifier.cpp (modified) (2 diffs)
• kjs/identifier.h (modified) (1 diff)
• kjs/property_map.cpp (modified) (2 diffs)
• kjs/ustring.cpp (modified) (8 diffs)
• kxmlcore/PassRefPtr.h (modified) (4 diffs)
• kxmlcore/RefPtr.h (modified) (1 diff)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2005-12-19  Maciej Stachowiak  <[email protected]>
+
+        Reviewed by Darin.
+
+        - fixed a leak in the assignment operator from PassRefPtr to RefPtr
+
+        * kxmlcore/RefPtr.h:
+        (KXMLCore::RefPtr::operator=):
+
+        - fix problem with PassRefPtr that darin spotted - it lacked a copy constructor
+        and therefore was using the default one, which can lead to excess derefs
+
+        I fixed this by adding a copy constructor from non-const
+        reference, and by adding a template pass() function that you have
+        to use when raw pointer or RefPtr are passed where PassRefPtr is
+        expected.
+
+        * kjs/identifier.cpp:
+        (KJS::Identifier::add): Changed to have PassRefPtr return type and
+        pass() the results.     
+        * kjs/identifier.h:
+        * kjs/property_map.cpp:
+        (KJS::PropertyMap::addSparseArrayPropertiesToReferenceList): Use pass()
+        where required.
+        * kjs/ustring.cpp:
+        (KJS::UString::UString): Use pass() as needed.
+        (KJS::UString::append): ditto
+        (KJS::UString::substr): ditto
+        * kjs/ustring.h:
+        (KJS::UString::UString): Use initializer instead of assignment
+        * kxmlcore/PassRefPtr.h:
+        (KXMLCore::PassRefPtr::PassRefPtr): Added copy constructor
+        (KXMLCore::pass): new template function to make it convenient to pass
+        a PassRefPtr
+
 2005-12-19  Geoffrey Garen  <[email protected]>
   

trunk/JavaScriptCore/kjs/identifier.cpp

@@ -124 +124 @@
 }
 
-UString::Rep *Identifier::add(const char *c)
+PassRefPtr<UString::Rep> Identifier::add(const char *c)
 {
     if (!c)
-        return &UString::Rep::null;
+        return pass(&UString::Rep::null);
     int length = strlen(c);
     if (length == 0)
-        return &UString::Rep::empty;
+        return pass(&UString::Rep::empty);
     
-    return *identifierTable().insert<const char *, hash, KJS::equal, convert>(c).first;
+    return pass(*identifierTable().insert<const char *, hash, KJS::equal, convert>(c).first);
 }
 
@@ -164 +164 @@
 }
 
-UString::Rep *Identifier::add(const UChar *s, int length)
+PassRefPtr<UString::Rep> Identifier::add(const UChar *s, int length)
 {
     if (length == 0)
-        return &UString::Rep::empty;
+        return pass(&UString::Rep::empty);
     
     UCharBuffer buf = {s, length}; 
-    return *identifierTable().insert<UCharBuffer, hash, KJS::equal, convert>(buf).first;
-}
-
-UString::Rep *Identifier::add(UString::Rep *r)
+    return pass(*identifierTable().insert<UCharBuffer, hash, KJS::equal, convert>(buf).first);
+}
+
+PassRefPtr<UString::Rep> Identifier::add(UString::Rep *r)
 {
     if (r->isIdentifier)
-        return r;
+        return pass(r);
 
     if (r->len == 0)
-        return &UString::Rep::empty;
+        return pass(&UString::Rep::empty);
 
     UString::Rep *result = *identifierTable().insert(r).first;
     if (result == r)
         r->isIdentifier = true;
-    return result;
+    return pass(result);
 }
 

trunk/JavaScriptCore/kjs/identifier.h

@@ -78 +78 @@
             { return equal(a._ustring.rep(), b); }
         
-        static UString::Rep *add(const char *);
-        static UString::Rep *add(const UChar *, int length);
-        static UString::Rep *add(UString::Rep *);
+        static PassRefPtr<UString::Rep> add(const char *);
+        static PassRefPtr<UString::Rep> add(const UChar *, int length);
+        static PassRefPtr<UString::Rep> add(UString::Rep *);
     };
     

trunk/JavaScriptCore/kjs/property_map.cpp

@@ -615 +615 @@
         UString::Rep *key = _singleEntry.key;
         if (key) {
-            UString k(key);
+            UString k(pass(key));
             bool fitsInUInt32;
             k.toUInt32(&fitsInUInt32);
             if (fitsInUInt32)
-                list.append(Reference(base, Identifier(key)));
+                list.append(Reference(base, Identifier(pass(key))));
         }
 #endif
@@ -630 +630 @@
         UString::Rep *key = entries[i].key;
         if (key && key != &UString::Rep::null) {
-            UString k(key);
+            UString k(pass(key));
             bool fitsInUInt32;
             k.toUInt32(&fitsInUInt32);
             if (fitsInUInt32)
-                list.append(Reference(base, Identifier(key)));
+                list.append(Reference(base, Identifier(pass(key))));
         }
     }

trunk/JavaScriptCore/kjs/ustring.cpp

@@ -200 +200 @@
   r->usedPreCapacity = 0;
   r->preCapacity = 0;
+
   // steal the single reference this Rep was created with
   return PassRefPtr<Rep>::adopt(r);
@@ -229 +230 @@
   r->usedPreCapacity = 0;
   r->preCapacity = 0;
+
   // steal the single reference this Rep was created with
   return PassRefPtr<Rep>::adopt(r);
@@ -458 +460 @@
     x.expandCapacity(aOffset + length);
     memcpy(const_cast<UChar *>(a.data() + aSize), b.data(), bSize * sizeof(UChar));
-    m_rep = Rep::create(a.m_rep, 0, length);
+    m_rep = Rep::create(pass(a.m_rep), 0, length);
   } else if (-bOffset == b.usedPreCapacity() && 4 * bSize >= aSize) {
     // - b reaches the beginning of its buffer so it qualifies for shared prepend
@@ -466 +468 @@
     y.expandPreCapacity(-bOffset + aSize);
     memcpy(const_cast<UChar *>(b.data() - aSize), a.data(), aSize * sizeof(UChar));
-    m_rep = Rep::create(b.m_rep, -aSize, length);
+    m_rep = Rep::create(pass(b.m_rep), -aSize, length);
   } else {
     // a does not qualify for append, and b does not qualify for prepend, gotta make a whole new string
@@ -684 +686 @@
     expandCapacity(thisOffset + length);
     memcpy(const_cast<UChar *>(data() + thisSize), t.data(), tSize * sizeof(UChar));
-    m_rep = Rep::create(m_rep, 0, length);
+    m_rep = Rep::create(pass(m_rep), 0, length);
   } else {
     // this is shared with someone using more capacity, gotta make a whole new string
@@ -725 +727 @@
     for (int i = 0; i < tSize; ++i)
       d[thisSize+i] = t[i];
-    m_rep = Rep::create(m_rep, 0, length);
+    m_rep = Rep::create(pass(m_rep), 0, length);
   } else {
     // this is shared with someone using more capacity, gotta make a whole new string
@@ -765 +767 @@
     UChar *d = const_cast<UChar *>(data());
     d[length] = c;
-    m_rep = Rep::create(m_rep, 0, length + 1);
+    m_rep = Rep::create(pass(m_rep), 0, length + 1);
   } else {
     // this is shared with someone using more capacity, gotta make a whole new string
@@ -1121 +1123 @@
     return *this;
 
-  return UString(Rep::create(m_rep, pos, len));
+  return UString(Rep::create(pass(m_rep), pos, len));
 }
 

trunk/JavaScriptCore/kxmlcore/PassRefPtr.h

@@ -45 +45 @@
         PassRefPtr(T *ptr) : m_ptr(ptr) { if (ptr) ptr->ref(); }
         PassRefPtr(const RefPtr<T>& o) : m_ptr(o.get()) { if (T *ptr = m_ptr) ptr->ref(); }
+        PassRefPtr(PassRefPtr& o) : m_ptr(o.release()) {}
 
         ~PassRefPtr() { if (T *ptr = m_ptr) ptr->deref(); }
@@ -84 +85 @@
             return *this;
         }
-      
+
         template<typename U>
         operator PassRefPtr_Ref<U>()
@@ -191 +192 @@
     }
 
+    template <typename T> inline PassRefPtr<T> pass(T *ptr)
+    {
+        return PassRefPtr<T>(ptr);
+    }
+
+    template <typename T> inline PassRefPtr<T> pass(const RefPtr<T>& ptr)
+    {
+        return PassRefPtr<T>(ptr);
+    }
+
 } // namespace KXMLCore
 
@@ -196 +207 @@
 using KXMLCore::static_pointer_cast;
 using KXMLCore::const_pointer_cast;
+using KXMLCore::pass;
 
 #endif // KXMLCORE_PASS_REF_PTR_H

trunk/JavaScriptCore/kxmlcore/RefPtr.h

@@ -91 +91 @@
     template <class T> inline RefPtr<T>& RefPtr<T>::operator=(PassRefPtr<T>& o) 
     {
-        T *optr = o.release();
-        if (optr)
-            optr->ref();
         if (T *ptr = m_ptr)
             ptr->deref();
-        m_ptr = optr;
+        m_ptr = o.release();
         return *this;
     }