[Gnash-commit] gnash ChangeLog libnet/cque.cpp

[Sandro Santilli]

CVSROOT:        /sources/gnash
Module name:    gnash
Changes by:     Sandro Santilli <strk>  08/04/07 20:25:15

Modified files:
        .              : ChangeLog 
        libnet         : cque.cpp 

Log message:
        implement safe ::merge method.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.6209&r2=1.6210
http://cvs.savannah.gnu.org/viewcvs/gnash/libnet/cque.cpp?cvsroot=gnash&r1=1.11&r2=1.12

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.6209
retrieving revision 1.6210
diff -u -b -r1.6209 -r1.6210
--- ChangeLog   7 Apr 2008 19:35:10 -0000       1.6209
+++ ChangeLog   7 Apr 2008 20:25:14 -0000       1.6210
@@ -1,3 +1,7 @@
+2008-04-07 Sandro Santilli <address@hidden>
+
+       * libnet/cque.cpp: implement safe ::merge method. 
+
 2008-04-07  Rob Savoye  <address@hidden>
 
        * libnet/cque.cpp: Only delete the pointer if we actually have

Index: libnet/cque.cpp
===================================================================
RCS file: /sources/gnash/gnash/libnet/cque.cpp,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -b -r1.11 -r1.12
--- libnet/cque.cpp     7 Apr 2008 19:35:11 -0000       1.11
+++ libnet/cque.cpp     7 Apr 2008 20:25:15 -0000       1.12
@@ -197,39 +197,62 @@
 amf::Buffer *
 CQue::merge(amf::Buffer *start)
 {
-    GNASH_REPORT_FUNCTION;
-    int totalsize = 0;
-    Que::iterator it;
-    vector<Que::iterator> elements; // iterators to elements to merge
-    vector<Que::iterator>::const_iterator eit;
-    Network::byte_t *ptr;
-    bool found = false;
-    
-    boost::mutex::scoped_lock lock(_mutex);
-    // Calculate how big the combined buffer needs to be. As all buffers by 
default
-    // are NETBUFSIZE, we know we're at the end of a merge because the final 
packet
-    // is less that the default.
-    for (it = _que.begin(); it != _que.end(); it++) {
-       amf::Buffer *ptr = *(it);
-       if ((ptr == start) || (found)) {
-           found = true;
-           if (ptr->size() == NETBUFSIZE) {
-               totalsize += ptr->size();
-               elements.push_back(it);
-           } else {            // add the last packet
-               totalsize += ptr->size();
-               elements.push_back(it);
-               break;
+    // Find iterator to first element to merge
+    Que::iterator from = std::find(_que.begin(), _que.end(), start); 
+    if ( from == _que.end() ) {
+        // Didn't find the requested Buffer pointer
+        return NULL;
            }
+
+
+    // Find iterator to last element to merge (first with size < NETBUFSIZE)
+    // computing total size with the same scan
+    size_t totalsize = (*from)->size();
+    Que::iterator to=from; ++to;
+    for (Que::iterator e=_que.end(); to!=e; ++to)
+    {
+        size_t sz = (*to)->size();
+        totalsize += sz;
+        if (sz < gnash::NETBUFSIZE) break;
        }
+    if ( to == _que.end() )
+    {
+        // Didn't find an element ending the merge
+        return NULL;
     }    
-    amf::Buffer *newbuf = new amf::Buffer(totalsize);
 
-    for (eit = elements.begin(); eit != elements.end(); eit++ ) {
-       newbuf->append(ptr);
-//     _que.erase(*eit);
+    // Merge all elements in a single buffer. We have totalsize now.
+    std::auto_ptr<amf::Buffer> newbuf ( new amf::Buffer(totalsize) );
+    Network::byte_t *tmp = newbuf->reference();
+    ++to;
+    for (Que::iterator i=from; i!=to; ++i)
+    {
+        amf::Buffer *buf = *i;
+        size_t sz = buf->size();
+        std::copy(buf->reference(), buf->reference() + sz, tmp);
+       //
+       // NOTE: If we're the buffer owners, it is safe to delete
+       //       the buffer now.
+       // delete buf;
+       //
+        tmp += sz;
     }
-    return newbuf;
+
+    // Finally erase all merged elements, and replace with the composite one
+    Que::iterator nextIter = _que.erase(from, to);
+
+    // NOTE: nextIter points now right after the last erased
+    // element. Sounds like a good place to put the new merged buffer
+    // to me, but the original code always did push_back so dunno 
+    // what's the correct behaviour
+    //
+    // Next line puts newbuf in place of the removed ones:
+    //
+    //  _que.insert(nextIter, newbuf);
+    //
+    _que.push_back(newbuf.get()); // <-- this is what the old code was doing
+
+    return newbuf.release(); // ownership is transferred. TODO: return auto_ptr
 }
     
 // Dump internal data.