Changeset 816

Timestamp:

07/30/08 20:04:36 (5 months ago)

Author:

mloskot

Message:

Fixed number of memory leaks (Ticket #63). Added detail::raii_wrapper class and used it to fix memory leaks in RAII way.

Location:

trunk

Files:

• include/liblas/detail/utility.hpp (modified) (1 diff)
• include/liblas/lasrecordheader.hpp (modified) (1 diff)
• src/detail/reader10.cpp (modified) (8 diffs)
• src/detail/reader11.cpp (modified) (7 diffs)
• src/detail/writer11.cpp (modified) (1 diff)
• src/lasheader.cpp (modified) (13 diffs)
• src/lasrecordheader.cpp (modified) (1 diff)

trunk/include/liblas/detail/utility.hpp

@@ -67 +67 @@
 namespace liblas { namespace detail {
 
+/// Simple RAII wrapper.
+/// It's dedicated to use with types associated with custom deleter,
+/// opaque pointers and C API objects.
+template <typename T>
+class raii_wrapper
+{
+    typedef void(*deleter_type)(T* p);
+
+public:
+
+    raii_wrapper(T* p, deleter_type d)
+        : p_(p), del_(d)
+    {
+        assert(0 != p_);
+        assert(0 != del_);
+    }
+
+    ~raii_wrapper()
+    {
+        do_delete(p_);
+    }
+
+    void reset(T* p)
+    {
+        do_delete(p_);
+        p_= p;
+    }
+
+    T* get() const
+    {
+        return p_;
+    }
+
+    void swap(raii_wrapper& other)
+    {
+        std::swap(p_, other.p_);
+    }
+
+private:
+
+    raii_wrapper(raii_wrapper const& other);
+    raii_wrapper& operator=(raii_wrapper const& rhs);
+
+    void do_delete(T* p)
+    {
+        assert(del_);
+        if (0 != p)
+            del_(p);
+    }
+
+    T* p_;
+    deleter_type del_;
+};
+
+
+/// Definition of variable-length record header.
 struct VLRHeader
 {

trunk/include/liblas/lasrecordheader.hpp

@@ -154 +154 @@
 
 #endif // LIBLAS_LASRECORDHEADER_HPP_INCLUDED
+

trunk/src/detail/reader10.cpp

@@ -46 +46 @@
 #include <liblas/lasheader.hpp>
 #include <liblas/laspoint.hpp>
-
+// GeoTIFF
 #ifdef HAVE_LIBGEOTIFF
 #include <geotiff.h>
@@ -53 +53 @@
 #include "geo_simpletags.h"
 #include "geovalues.h"
-#endif /* HAVE_LIBGEOTIFF */
-
+#endif // HAVE_LIBGEOTIFF
 // std
 #include <fstream>
 #include <iostream>
-#include <cstdlib> // std::size_t
+#include <stdexcept>
+#include <cstdlib> // std::size_t, std::free
 #include <cassert>
-#include <stdexcept>
 
 namespace liblas { namespace detail { namespace v10 {
@@ -214 +213 @@
 }
 
-bool ReaderImpl::ReadVLR(LASHeader& header) {
-    
+bool ReaderImpl::ReadVLR(LASHeader& header)
+{
     VLRHeader vlrh = { 0 };
 
@@ -250 +249 @@
 #ifndef HAVE_LIBGEOTIFF
     UNREFERENCED_PARAMETER(header);
-    return false;
 #else
-    // TODO: Under construction
 
     std::string const uid("LASF_Projection");
-    ST_TIFF *st = ST_Create();
+
+    detail::raii_wrapper<ST_TIFF> st(ST_Create(), ST_Destroy);
 
     for (uint16_t i = 0; i < header.GetRecordsCount(); ++i)
@@ -264 +262 @@
         {
             int16_t count = data.size()/sizeof(int16_t);
-            ST_SetKey( st, record.GetRecordId(), count, STT_SHORT, 
-                       &(data[0]) );
+            ST_SetKey(st.get(), record.GetRecordId(), count, STT_SHORT, &(data[0]));
         }
 
@@ -271 +268 @@
         {
             int count = data.size() / sizeof(double);
-            ST_SetKey( st, record.GetRecordId(), count, STT_DOUBLE, 
-                       &(data[0]) );
+            ST_SetKey(st.get(), record.GetRecordId(), count, STT_DOUBLE, &(data[0]));
         }        
 
@@ -278 +274 @@
         {
             uint8_t count = data.size()/sizeof(uint8_t);
-            ST_SetKey( st, record.GetRecordId(), count, STT_ASCII, 
-                       &(data[0]) );
-        }
-    }
-
-    if (st->key_count) {
-        GTIF *gtif = GTIFNewSimpleTags( st );
+            ST_SetKey(st.get(), record.GetRecordId(), count, STT_ASCII, &(data[0]));
+        }
+    }
+
+    if (st.get()->key_count)
+    {
+        raii_wrapper<GTIF> gtif(GTIFNewSimpleTags(st.get()), GTIFFree);
+
         GTIFDefn defn;
-        if (GTIFGetDefn(gtif, &defn)) 
-        {
-            header.SetProj4(std::string(GTIFGetProj4Defn(&defn)));
-        }
-
-// #ifdef DEBUG
-//         printf("Geotiff from reader...\n");
-//         GTIFPrint(gtif, 0, 0);
-// #endif
-// 
-        GTIFFree( gtif );
-        ST_Destroy( st );
+        if (GTIFGetDefn(gtif.get(), &defn)) 
+        {
+            char* proj4def = GTIFGetProj4Defn(&defn);
+            std::string tmp(proj4def);
+            std::free(proj4def);
+
+            header.SetProj4(tmp);
+        }
+
         return true;
-    } else {
-        return false;
-    }
+    }
+
 #endif /* def HAVE_LIBGEOTIFF */
+
+    return false;
 }
 
@@ -394 +389 @@
 
 }}} // namespace liblas::detail::v10
+

trunk/src/detail/reader11.cpp

@@ -46 +46 @@
 #include <liblas/laspoint.hpp>
 #include <liblas/lasrecordheader.hpp>
-
-
+// GeoTIFF
 #ifdef HAVE_LIBGEOTIFF
 #include <geotiff.h>
@@ -54 +53 @@
 #include "geo_simpletags.h"
 #include "geovalues.h"
-#endif /* HAVE_LIBGEOTIFF */
-
+#endif //  HAVE_LIBGEOTIFF
 // std
 #include <fstream>
+#include <stdexcept>
 #include <cstdlib> // std::size_t
 #include <cassert>
-#include <stdexcept>
 
 namespace liblas { namespace detail { namespace v11 {
@@ -306 +304 @@
 }
 
-bool ReaderImpl::ReadVLR(LASHeader& header) {
-    
+bool ReaderImpl::ReadVLR(LASHeader& header)
+{
     VLRHeader vlrh = { 0 };
 
@@ -341 +339 @@
 #ifndef HAVE_LIBGEOTIFF
     UNREFERENCED_PARAMETER(header);
-    return false;
 #else
-    // TODO: Under construction
 
     std::string const uid("LASF_Projection");
-    ST_TIFF *st = ST_Create();
+
+    detail::raii_wrapper<ST_TIFF> st(ST_Create(), ST_Destroy);
 
     for (uint16_t i = 0; i < header.GetRecordsCount(); ++i)
@@ -355 +352 @@
         {
             int16_t count = data.size()/sizeof(int16_t);
-            ST_SetKey( st, record.GetRecordId(), count, STT_SHORT, 
-                       &(data[0]) );
+            ST_SetKey(st.get(), record.GetRecordId(), count, STT_SHORT, &(data[0]));
         }
 
@@ -362 +358 @@
         {
             int count = data.size() / sizeof(double);
-            ST_SetKey( st, record.GetRecordId(), count, STT_DOUBLE, 
-                       &(data[0]) );
+            ST_SetKey(st.get(), record.GetRecordId(), count, STT_DOUBLE, &(data[0]));
         }        
 
@@ -369 +364 @@
         {
             uint8_t count = data.size()/sizeof(uint8_t);
-            ST_SetKey( st, record.GetRecordId(), count, STT_ASCII, 
-                       &(data[0]) );
+            ST_SetKey(st.get(), record.GetRecordId(), count, STT_ASCII, &(data[0]));
         }
     }
 
-    if (st->key_count) {
-        GTIF *gtif = GTIFNewSimpleTags( st );
+    if (st.get()->key_count)
+    {
+        raii_wrapper<GTIF> gtif(GTIFNewSimpleTags(st.get()), GTIFFree);
+
         GTIFDefn defn;
-        if (GTIFGetDefn(gtif, &defn)) 
-        {
-            header.SetProj4(std::string(GTIFGetProj4Defn(&defn)));
+        if (GTIFGetDefn(gtif.get(), &defn)) 
+        {
+            char* proj4def = GTIFGetProj4Defn(&defn);
+            std::string tmp(proj4def);
+            std::free(proj4def);
+
+            header.SetProj4(tmp);
         }
-        GTIFFree( gtif );
-        ST_Destroy( st );
+        
         return true;
-    } else {
-        return false;
-    }
+    }
+
 #endif /* def HAVE_LIBGEOTIFF */
+
+    return false;
 }
 
 }}} // namespace liblas::detail::v11
+

trunk/src/detail/writer11.cpp

@@ -276 +276 @@
 
 }}} // namespace liblas::detail::v11
+

trunk/src/lasheader.cpp

@@ -45 +45 @@
 #include <liblas/guid.hpp>
 #include <liblas/detail/utility.hpp>
+// GeoTIFF
+#ifdef HAVE_LIBGEOTIFF
+#include <geotiff.h>
+#include <geo_simpletags.h>
+#include "geo_normalize.h"
+#include "geo_simpletags.h"
+#include "geovalues.h"
+#include "geotiffio.h"
+#endif // HAVE_LIBGEOTIFF
 //std
 #include <algorithm>
@@ -53 +62 @@
 #include <cstring> // std::memset, std::memcpy, std::strncpy
 #include <cassert>
-#include <time.h>
-
-
-#ifdef HAVE_LIBGEOTIFF
-#include <geotiff.h>
-#include <geo_simpletags.h>
-#include "geo_normalize.h"
-#include "geo_simpletags.h"
-#include "geovalues.h"
-#include "geotiffio.h"   /* public interface        */
-#endif /* HAVE_LIBGEOTIFF */
+#include <ctime>
+
 
 namespace liblas
@@ -585 +585 @@
 
 
-    time_t now;
-    tm *ptm;
-
-    time(&now);
-    ptm = gmtime(&now);
+    std::time_t now;
+    std::tm *ptm;
+
+    std::time(&now);
+    ptm = std::gmtime(&now);
     
     m_createDOY = static_cast<uint16_t>(ptm->tm_yday);
@@ -624 +624 @@
 void LASHeader::ClearGeoKeyVLRs()
 {
-
     std::string const uid("LASF_Projection");
 
@@ -639 +638 @@
         beg_size += (*i).GetTotalSize();
 
-        uint16_t id = record.GetRecordId();
-        const char* user = record.GetUserId(true).c_str();
-        
-        if (uid == user && 34735 == id)
+        std::string user = record.GetUserId(true);
+        if (uid == user.c_str())
         {
-            // Geotiff SHORT key
-            for(j = vlrs.begin(); j != vlrs.end(); ++j) {
-                if (*j == *i) {
-                    vlrs.erase(j);
-                    break;
+            uint16_t id = record.GetRecordId();
+
+            if (34735 == id)
+            {
+                // Geotiff SHORT key
+                for(j = vlrs.begin(); j != vlrs.end(); ++j)
+                {
+                    if (*j == *i)
+                    {
+                        vlrs.erase(j);
+                        break;
+                    }
                 }
             }
-        }
-
-        if (uid == user && 34736 == id)
-        {
-            // Geotiff DOUBLE key
-            for(j = vlrs.begin(); j != vlrs.end(); ++j) {
-                if (*j == *i) {
-                    vlrs.erase(j);
-                    break;
+            else if (34736 == id)
+            {
+                // Geotiff DOUBLE key
+                for(j = vlrs.begin(); j != vlrs.end(); ++j)
+                {
+                    if (*j == *i)
+                    {
+                        vlrs.erase(j);
+                        break;
+                    }
+                }
+            }        
+            else if (34737 == id)
+            {
+                // Geotiff ASCII key
+                for (j = vlrs.begin(); j != vlrs.end(); ++j)
+                {
+                    if (*j == *i)
+                    {
+                        vlrs.erase(j);
+                        break;
+                    }
                 }
             }
-        }        
-
-        if (uid == user && 34737 == id)
-        {
-            // Geotiff ASCII key
-            for(j = vlrs.begin(); j != vlrs.end(); ++j) {
-                if (*j == *i) {
-                    vlrs.erase(j);
-                    break;
-                }
-            }
-        }
+        } // uid == user
     }
     
@@ -691 +697 @@
     
     // Add the signature if we're a 1.0 file    
-    if (eVersionMinorMin == m_versionMinor) {
+    if (eVersionMinorMin == m_versionMinor)
+    {
         size = end_size + dataSignatureSize; 
-    } else {
+    }
+    else
+    {
         size = end_size;
     }
@@ -704 +713 @@
 #ifndef HAVE_LIBGEOTIFF
 
+    ;
+
 #else
+
     int ret = 0;
     
-        // TODO - mloskot: probable lack of RAII here.
-        // Who is the owner of st and gt pointees?
-
-        ST_TIFF* st = ST_Create();
-    assert(0 != st);
-
-        GTIF* gt = GTIFNewSimpleTags( st );    
-    assert(0 != gt);
+    detail::raii_wrapper<ST_TIFF> st(ST_Create(), ST_Destroy);
+    detail::raii_wrapper<GTIF> gt(GTIFNewSimpleTags(st.get()), GTIFFree);    
         
     // Wipe out any existing VLRs that represent geotiff keys on the 
@@ -724 +730 @@
                 return;
 
-    ret = GTIFSetFromProj4(gt, GetProj4().c_str());
+    ret = GTIFSetFromProj4(gt.get(), GetProj4().c_str());
     if (!ret)
         {
@@ -730 +736 @@
     }
 
-    ret = GTIFWriteKeys(gt);
+    ret = GTIFWriteKeys(gt.get());
     if (!ret)
         {
@@ -737 +743 @@
     
     short* kdata = NULL;
-    short kvalue;
-    double dvalue;
+    short kvalue = 0;
     double* ddata = NULL;
-    char* adata = NULL;
-    char* avalue = NULL;
-    int atype, dtype, ktype, acount, dcount, kcount;
+    double dvalue = 0;
+    int dtype = 0;
+    int dcount = 0;
+    int ktype = 0;
+    int kcount = 0;
 
     //GTIFF_GEOKEYDIRECTORY == 34735
-    ret = ST_GetKey(st, 34735, &kcount, &ktype, (void**)&kdata);
+    ret = ST_GetKey(st.get(), 34735, &kcount, &ktype, (void**)&kdata);
     if (ret)
         {    
@@ -777 +784 @@
     // FIXME We don't handle ASCII keys yet
     // GTIFF_ASCIIPARAMS == 34737
-    // ret = ST_GetKey(st, 34737, &acount, &atype, (void**)&adata);
+    // ret = ST_GetKey(st.get(), 34737, &acount, &atype, (void**)&adata);
     // if (ret) {
     //     
@@ -812 +819 @@
 
     // GTIFF_DOUBLEPARAMS == 34736
-    ret = ST_GetKey(st, 34736, &dcount, &dtype, (void**)&ddata);
+    ret = ST_GetKey(st.get(), 34736, &dcount, &dtype, (void**)&ddata);
 
     if (ret)
@@ -849 +856 @@
 }
 
-
 } // namespace liblas
+

trunk/src/lasrecordheader.cpp

@@ -219 +219 @@
 
 }
+
 } // namespace liblas
+