[Gnash-commit] /srv/bzr/gnash/trunk r9554: Give dump gui the C++ treatment. Minor cleanups to extention.cpp, comment

[Benjamin Wolsey]

------------------------------------------------------------
revno: 9554
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Sun 2008-08-03 12:48:41 +0200
message:
  Give dump gui the C++ treatment. Minor cleanups to extention.cpp, comment
  on implementation of edit_text_character.cpp.
modified:
  gui/dump.cpp
  gui/dump.h
  libbase/extension.cpp
  libcore/edit_text_character.cpp
    ------------------------------------------------------------
    revno: 9553.1.1
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Sun 2008-08-03 11:16:22 +0200
    message:
      Don't allocate ofstream on the heap. Don't close ofstream: it closes when
      it goes out of scope. Initialize class members in initialization list.
      Use new instead of malloc, catch std::bad_alloc. Use boost::scoped_array
      for exception safety. Use std::string instead of char arrays.
    modified:
      gui/dump.cpp
      gui/dump.h
    ------------------------------------------------------------
    revno: 9553.1.2
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Sun 2008-08-03 11:19:06 +0200
    message:
      Indentation.
    modified:
      libbase/extension.cpp
    ------------------------------------------------------------
    revno: 9553.1.3
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Sun 2008-08-03 12:42:46 +0200
    message:
      Notes on string iterator.
    modified:
      libcore/edit_text_character.cpp

=== modified file 'gui/dump.cpp'
--- a/gui/dump.cpp      2008-05-01 15:30:17 +0000
+++ b/gui/dump.cpp      2008-08-03 09:16:22 +0000
@@ -37,6 +37,7 @@
 
 #include <iostream>
 #include <string>
+#include <fstream>
 
 #ifndef RENDERER_AGG
 #error Dump gui requires AGG renderer
@@ -77,39 +78,28 @@
 }
 #endif
 
+// TODO:  Let user decide bits-per-pixel
+// TODO:  let user decide colorspace (see also _bpp above!)
 DumpGui::DumpGui(unsigned long xid, float scale, bool loop, unsigned int 
depth) :
-    Gui(xid, scale, loop, depth)
+    Gui(xid, scale, loop, depth),
+    _agg_renderer(NULL),
+    _offscreenbuf(NULL),
+    _offscreenbuf_size(-1),
+    _timeout(0),
+    _framecount(0),
+    _bpp(32),
+    _pixelformat("BGRA32")
 {
-
-    _agg_renderer = NULL;
-    _offscreenbuf = NULL;
-    _offscreenbuf_size = -1;
-    _file_output = NULL;
-    _file_stream = NULL;
-    _timeout = 0;
-    _framecount = 0;
-    _bpp = 32;                  // TODO:  Let user decide bits-per-pixel
-
-    // TODO:  let user decide colorspace (see also _bpp above!)
-    strncpy(_pixelformat, "BGRA32", sizeof(_pixelformat));
     if (loop) {
-        std::cout << "# WARNING:  Gnash was told to loop the movie" << 
std::endl;
+        std::cerr << "# WARNING:  Gnash was told to loop the movie" << 
std::endl;
     }
     if (_xid) {
-        std::cout << "# WARNING:  Ignoring request to display in X11 window" 
<< std::endl;
+        std::cerr << "# WARNING:  Ignoring request to display in X11 window" 
<< std::endl;
     }
 }
 
 DumpGui::~DumpGui()
 {
-    if (_offscreenbuf) {
-        free(_offscreenbuf);
-        _offscreenbuf=NULL;
-    }
-    if ((DumpGui::_file_stream != NULL) && (DumpGui::_file_stream->is_open())) 
{
-        DumpGui::_file_stream->close();
-        std::cout << "# Finished writing file" << std::endl;
-    }
     std::cout << "FRAMECOUNT=" << _framecount << "" << std::endl;
 }
 
@@ -128,14 +118,19 @@
     opterr = 0;
     while ((c = getopt (argc, *argv, "D:")) != -1) {
         if (c == 'D') {
-            _file_output = optarg;
+            // Terminate if no filename is given.
+            if (!optarg) {
+                std::cout << 
+                    _("# FATAL:  No filename given with -D argument.") << 
std::endl;      
+                return false;
+            }      
+            _fileOutput = optarg;
         }
     }
     opterr = origopterr;
 
-    if (_file_output == NULL) {
-        std::cout << 
-            _("# FATAL:  No filename given with -D argument.") << std::endl;
+    if (_fileOutput.empty()) {
+        // Terminate if filename is empty.
         return false;
     }
 
@@ -145,7 +140,7 @@
 #endif
 
     init_dumpfile();
-    _agg_renderer = create_render_handler_agg(_pixelformat);
+    _agg_renderer = create_render_handler_agg(_pixelformat.c_str());
     set_render_handler(_agg_renderer);
 
     return true;
@@ -249,12 +244,12 @@
 void
 DumpGui::writeFrame()
 {
-    assert(_file_stream != NULL);
-    if (_file_stream->is_open()) {
-        _file_stream->write((char*)_offscreenbuf, _offscreenbuf_size);
-        //_file_stream->flush();
+    assert(_fileStream);
+    if (_fileStream.is_open()) {
+        _fileStream.write(reinterpret_cast<char*>(_offscreenbuf.get()), 
_offscreenbuf_size);
         _framecount++;
-    } else {
+    }
+    else {
         std::cout << _("# FATAL:  Unable to write to closed output file.") << 
"" << std::endl;
         log_error(_("Unable to write to closed output file."));
         quit();
@@ -264,71 +259,69 @@
 void
 DumpGui::init_dumpfile()
 {
-    if (_file_output == NULL) {
-        log_error(_("Please supply a dump filename for gnash-dump."));
+    // This should never be empty.
+    assert (!_fileOutput.empty());
+
+    _fileStream.open(_fileOutput.c_str());
+    
+    if (!_fileStream) {
+        log_error(_("Unable to write file '%s'."), _fileOutput);
+        std::cerr << "# FATAL:  Unable to write file '" << _fileOutput << "'" 
<< std::endl;
         exit(1);
-    } else {
-        _file_stream = new std::ofstream();
-        _file_stream->open(_file_output);
-        if (_file_stream->fail()) {
-            log_error(_("Unable to write file '%s'."), _file_output);
-            std::cout << "# FATAL:  Unable to write file '" << _file_output << 
"'" << std::endl;
-            exit(1);
-        } else {
-            // Yes, this should go to cout.  The user needs to know this
-            // information in order to process the file.  Print out in a
-            // format that is easy to source into shell.
-            std::cout << 
-                "# Gnash created a raw dump file with the following 
properties:" << std::endl <<
-                "COLORSPACE=" << _pixelformat << std::endl <<
-                "NAME=" << _file_output  << std::endl;
-        }
     }
+    
+    // Yes, this should go to cout.  The user needs to know this
+    // information in order to process the file.  Print out in a
+    // format that is easy to source into shell.
+    std::cout << 
+        "# Gnash created a raw dump file with the following properties:" << 
std::endl <<
+        "COLORSPACE=" << _pixelformat << std::endl <<
+        "NAME=" << _fileOutput  << std::endl;
+    
 }
 
 void
 DumpGui::setRenderHandlerSize(int width, int height)
 {
-    assert(width>0);
-    assert(height>0);
-    assert(_agg_renderer!=NULL);
+    assert(width > 0);
+    assert(height > 0);
+    assert(_agg_renderer != NULL);
     
-    if ((_offscreenbuf != NULL) && (width == _width) && (height == _height))
+    if (_offscreenbuf.get() && (width == _width) && (height == _height)) {
         return;
+    }
           
     _width = width;
     _height = height;
     std::cout << "WIDTH=" << _width  << std::endl <<
         "HEIGHT=" << _height  << std::endl;
-    if (_offscreenbuf) {  
-        free(_offscreenbuf);
-        _offscreenbuf = NULL;
-    }
 
     int row_size = width*((_bpp+7)/8);
-    int new_bufsize = row_size * height;
+    int newBufferSize = row_size * height;
        
-    // TODO: At the moment we only increase the buffer and never decrease it. 
Should be
-    // changed sometime; movies which change the stage size will probably cause
-    // all sorts of havok with raw data, but that's not our problem...
-    if (new_bufsize > _offscreenbuf_size) {
-        // TODO: C++ conform alternative to realloc?
-        _offscreenbuf  = static_cast<unsigned char *>( realloc(_offscreenbuf, 
new_bufsize) );
-        if (!_offscreenbuf) {
-            log_debug("Could not allocate %i bytes for offscreen buffer: %s",
-                      new_bufsize, strerror(errno)
-                      );
-            return;
-        }
-  
-        log_debug("DUMP-AGG: %i bytes offscreen buffer allocated", 
new_bufsize);
-  
-        _offscreenbuf_size = new_bufsize;
-        memset(_offscreenbuf, 0, new_bufsize);
+    // Reallocate the buffer when it shrinks or grows.
+    if (newBufferSize != _offscreenbuf_size) {
+
+        try {
+              _offscreenbuf.reset(new unsigned char[newBufferSize]);
+              log_debug("DUMP-AGG: %i bytes offscreen buffer allocated", 
newBufferSize);
+        }
+        catch (std::bad_alloc &e)
+        {
+            log_error("Could not allocate %i bytes for offscreen buffer: %s",
+                  newBufferSize, e.what());
+                  
+              // TODO: what to do here? An assertion in render_handler_agg.cpp
+              // fails if we just return.
+              return;
+        }
+  
+        _offscreenbuf_size = newBufferSize;
+
     }
-       
+
     static_cast<render_handler_agg_base *> (_agg_renderer)->init_buffer
-        (_offscreenbuf,
+        (_offscreenbuf.get(),
          _offscreenbuf_size,
          _width,
          _height,

=== modified file 'gui/dump.h'
--- a/gui/dump.h        2008-05-02 12:50:37 +0000
+++ b/gui/dump.h        2008-08-03 09:16:22 +0000
@@ -15,8 +15,8 @@
 // along with this program; if not, write to the Free Software
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 
-#ifndef __DUMP_H__
-#define __DUMP_H__
+#ifndef GNASH_DUMP_H
+#define GNASH_DUMP_H
 
 #ifdef HAVE_CONFIG_H
 #include "gnashconfig.h"
@@ -24,6 +24,8 @@
 
 #include "dsodefs.h" // for DSOEXPORT
 #include "gui.h" // for inheritance
+#include <fstream>
+#include <boost/scoped_array.hpp>
 
 namespace gnash
 {
@@ -61,17 +63,28 @@
     bool want_redraw() { return false; }
     void writeFrame();
 
- private:
-    unsigned int _bpp;                  /* bits per pixel */
-    char _pixelformat[16];              /* colorspace name (eg, "RGB24") */
-    char* _file_output;                 /* path to output file */
-    std::ofstream* _file_stream;        /* stream for output file */
+private:
+    render_handler *_agg_renderer;      /* pointer to AGG renderer */
+
+    // A buffer to hold the actual image data. A boost::scoped_array
+    // is destroyed on reset and when it goes out of scope (including on
+    // stack unwinding after an exception), so there is no need to delete
+    // it.
+    boost::scoped_array<unsigned char> _offscreenbuf;
+
+    int _offscreenbuf_size;             /* size of window (bytes) */
+
     unsigned int _timeout;              /* maximum length of movie */
     unsigned int _framecount;           /* number of frames rendered */
+
+    unsigned int _bpp;                  /* bits per pixel */
+    std::string _pixelformat;              /* colorspace name (eg, "RGB24") */
+
+    std::string _fileOutput;                 /* path to output file */
+    std::ofstream _fileStream;        /* stream for output file */
     void init_dumpfile();               /* convenience method to create dump 
file */
-    render_handler *_agg_renderer;      /* pointer to AGG renderer */
-    unsigned char *_offscreenbuf;       /* our "window" */
-    int _offscreenbuf_size;             /* size of window (bytes) */
+
+
 
 };
 

=== modified file 'libbase/extension.cpp'
--- a/libbase/extension.cpp     2008-07-30 16:34:59 +0000
+++ b/libbase/extension.cpp     2008-08-03 09:19:06 +0000
@@ -24,7 +24,7 @@
 // #if defined(_WIN32) || defined(WIN32)
 // # define lock(lib_mutex);
 // # define scoped_lock ;
-// #define PLUGINSDIR "./"     //hack
+// #define PLUGINSDIR "./"    //hack
 // #define USE_DIRENT 1
 // #else
 // # include <boost/detail/lightweight_mutex.hpp>
@@ -43,7 +43,7 @@
 #include "extension.h"
 #include "as_object.h"
 
-#if HAVE_DIRENT_H || WIN32==1  // win32 hack
+#if HAVE_DIRENT_H || WIN32==1    // win32 hack
 # include <dirent.h>
 # define NAMLEN(dirent) std::strlen((dirent)->d_name)
 #else
@@ -139,7 +139,8 @@
         sl = new SharedLib(module);
         sl->openLib();
         _plugins[module] = sl;
-    } else {
+    }
+    else {
         sl = _plugins[module];
     }
     
@@ -149,7 +150,8 @@
 
     if (symptr) {    
         symptr(where);
-    } else {
+    }
+    else {
         log_error(_("Couldn't get class_init symbol"));
     }
     
@@ -158,29 +160,31 @@
 
 bool
 Extension::initModuleWithFunc(const std::string& module, const std::string& 
func,
-       as_object &obj)
+    as_object &obj)
 {
-       SharedLib *sl;
-
-       log_security(_("Initializing module: \"%s\""), module);
-
-       if (_plugins[module] == 0) {
-               sl = new SharedLib(module);
-               sl->openLib();
-               _plugins[module] = sl;
-       } else {
-               sl = _plugins[module];
-       }
-
-       SharedLib::initentry *symptr = sl->getInitEntry(func);
-
-       if (symptr) {
-               symptr(obj);
-       } else {
-               log_error(_("Couldn't get class_init symbol: \"%s\""), func);
-       }
-
-       return true;
+    SharedLib *sl;
+
+    log_security(_("Initializing module: \"%s\""), module);
+
+    if (_plugins[module] == 0) {
+        sl = new SharedLib(module);
+        sl->openLib();
+        _plugins[module] = sl;
+    }
+    else {
+        sl = _plugins[module];
+    }
+
+    SharedLib::initentry *symptr = sl->getInitEntry(func);
+
+    if (symptr) {
+        symptr(obj);
+    }
+    else {
+        log_error(_("Couldn't get class_init symbol: \"%s\""), func);
+    }
+
+    return true;
 }
 
 bool
@@ -197,8 +201,8 @@
 //    GNASH_REPORT_FUNCTION;
     
     Tok t(dirlist, Sep(":"));
-    for (Tok::iterator i = t.begin(), e = t.end(); i != e; ++i)
-    {
+    for (Tok::iterator i = t.begin(), e = t.end(); i != e; ++i) {
+
         const std::string& dir = *i;
 
         log_debug(_("Scanning directory \"%s\" for plugins"), dir);
@@ -207,9 +211,10 @@
         if (!libdir) {
             log_error(_("Can't open directory %s"), dir);
             return false;
-       }   
+        }   
         
         struct dirent *entry = readdir(libdir);
+
         for (int i = 0; entry > 0; ++i) {
             // We only want shared libraries that end with the suffix, 
otherwise
             // we get all the duplicates.
@@ -219,23 +224,25 @@
                 continue;
             }
 
-           std::string name(entry->d_name);
+            std::string name(entry->d_name);
 
-           // Hidden files.
+            // Hidden files.
             if (name.at(0) == '.') {
                 continue;
             }            
-            
-           const std::string::size_type pos = name.find_last_of('.');
-           if (pos == std::string::npos) continue;
-           const std::string suffix = name.substr(pos);
-           name.erase(pos);
+                
+            const std::string::size_type pos = name.find_last_of('.');
+ 
+            if (pos == std::string::npos) continue;
+ 
+            const std::string suffix = name.substr(pos);
+            name.erase(pos);
 
             if (suffix == ".so") {
                 log_debug(_("Gnash Plugin name: %s"), name);
                 _modules.push_back(name);
             }
-           else {
+            else {
                 continue;
             }
         }

=== modified file 'libcore/edit_text_character.cpp'
--- a/libcore/edit_text_character.cpp   2008-07-22 17:16:51 +0000
+++ b/libcore/edit_text_character.cpp   2008-08-03 10:42:46 +0000
@@ -1285,6 +1285,12 @@
        assert(! _text.empty() );
        
        boost::uint32_t code = 0;
+       
+       // String iterators are very sensitive to 
+       // potential changes to the string (to allow for copy-on-write).
+       // So there must be no external changes to the string or
+       // calls to most non-const member functions during this loop.
+       // Especially not c_str() or data().
     std::wstring::const_iterator it = _text.begin();
     const std::wstring::const_iterator e = _text.end();