fix: use managed memory for Run’s _text member

This removes the need to manually manage memory for this member but, more
importantly makes this class copy-safe. The implicit copy constructor of this
class would copy the pointer in the _text member. When either the original
object or the new copy was deleted, the pointer would be freed leaving its
duplicate in the other copy dangling. Any attempt to access this would result in
use-after-free and deleting the object would cause a double-free.

Following this change, it is safe to copy a Run object. This issue was latent,
because no existing code causes a Run object to be copied.

diff --git a/plugin/visio/VisioText.cpp b/plugin/visio/VisioText.cpp
index 3417c36b6e4ab164c2c8ca0071f7dc003d84e159..0912b2d40bb5129c5bf29341196acfeafc9105b1 100644 (file)
--- a/plugin/visio/VisioText.cpp
+++ b/plugin/visio/VisioText.cpp
@@ -17,7 +17,9 @@
 #include <cstdlib>
 #include <string.h>
 
-extern "C" char *xml_string(char* str);
+// slight lie that this function takes a const pointer (it does not, but we know
+// it does not modify its argument)
+extern "C" char *xml_string(const char* str);
 
 namespace Visio
 {
@@ -51,18 +53,12 @@ namespace Visio
                gvputs(job, "</Para>\n");
        }
        
-       Run::Run(boxf bounds, char* text):
+       Run::Run(boxf bounds, const char* text):
                _bounds(bounds),
-               _text(strdup(text))     /* copy text */
+               _text(text)     /* copy text */
        {
        }
        
-       Run::~Run()
-       {
-               /* since we copied, we need to free */
-               free(_text);
-       }
-       
        boxf Run::GetBounds() const
        {
                return _bounds;
@@ -70,7 +66,7 @@ namespace Visio
        
        void Run::Print(GVJ_t* job, unsigned int index) const
        {
-               gvprintf(job, "<pp IX='%d'/><cp IX='%d'/>%s\n", index, index, _text ? xml_string(_text) : "");  /* para mark + char mark + actual text */
+               gvprintf(job, "<pp IX='%d'/><cp IX='%d'/>%s\n", index, index, xml_string(_text.c_str()));       /* para mark + char mark + actual text */
        }
        
        Text* Text::CreateText(GVJ_t* job, pointf p, textspan_t* span)

diff --git a/plugin/visio/VisioText.h b/plugin/visio/VisioText.h
index 7e71ffe9dea7f0ef3930d21d7fe299ac98dd45e3..ffe56864f8a10ccd907e566829161f0c3d4876c0 100644 (file)
--- a/plugin/visio/VisioText.h
+++ b/plugin/visio/VisioText.h
@@ -11,6 +11,7 @@
 #pragma once
 
 #include <common/types.h>
+#include <string>
 
 namespace Visio
 {
@@ -59,8 +60,7 @@ namespace Visio
        class Run
        {
        public:
-               Run(boxf bounds, char* text);
-               ~Run();
+               Run(boxf bounds, const char* text);
                
                boxf GetBounds() const;         /* bounding box -- used by text logic */
                
@@ -69,7 +69,7 @@ namespace Visio
                
        private:
                boxf _bounds;
-               char* _text;
+               std::string _text;
        };
        
        /* Para, Char and Run details for each Graphviz text */