From 5237185e8ce72d18dce0dd7764fe11970d83e586 Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Tue, 21 Apr 2015 17:29:15 +0000 Subject: [PATCH] From Chris Denham, "I found a couple of memory leaks in 3DS reader plugin and I have attached corrected files. I have attached an example 3DS file I used to test the leaks/fixes using osgviewer for trunk at rev [14853] and the tagged version 3.2.1. The first leak is in the lib3ds module (yeah, I know that probably should be corrected at http://code.google.com/p/lib3ds/ but I'm assuming that as no commits have happened there since 2011 that it may be better to fix the copy we have in the OSG of that project) The leak is caused by lib3d's use of realloc(ptr, 0) to free up memory allocations, but realloc, when ptr==NULL returns malloc(0) rather than NULL and thus leaks a zero byte allocation. The solution here was to adjust the 'lib3ds_util_reserve_array' function so that it realloc is not used to release a NULL pointer. The second leak is in ReaderWriter3DS.cpp and arises when any of the created StateSet objects added to the StateSetMap don't subsequently get applied to a Node. The solution here was just to simply use the osg::ref_ptr around the raw StateSet pointer that was used in the locally defined StateSetInfo struct." git-svn-id: http://svn.openscenegraph.org/osg/OpenSceneGraph/trunk@14854 16af8721-9629-0410-8352-f15c8da7e697 --- src/osgPlugins/3ds/ReaderWriter3DS.cpp | 6 +++--- src/osgPlugins/3ds/lib3ds/lib3ds_util.c | 11 ++++++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/osgPlugins/3ds/ReaderWriter3DS.cpp b/src/osgPlugins/3ds/ReaderWriter3DS.cpp index 1615bf5af..5287fedf7 100644 --- a/src/osgPlugins/3ds/ReaderWriter3DS.cpp +++ b/src/osgPlugins/3ds/ReaderWriter3DS.cpp @@ -193,7 +193,7 @@ protected: StateSetInfo(const StateSetInfo & v) : stateset(v.stateset), lib3dsmat(v.lib3dsmat) {} StateSetInfo & operator=(const StateSetInfo & v) { stateset=v.stateset; lib3dsmat=v.lib3dsmat; return *this; } - osg::StateSet * stateset; + osg::ref_ptr stateset; Lib3dsMaterial * lib3dsmat; }; @@ -413,7 +413,7 @@ void ReaderWriter3DS::ReaderObject::addDrawableFromFace(osg::Geode * geode, Face if (drawable.valid()) { if (ssi.stateset) - drawable->setStateSet(ssi.stateset); + drawable->setStateSet(ssi.stateset.get()); geode->addDrawable(drawable.get()); } } @@ -426,7 +426,7 @@ void ReaderWriter3DS::ReaderObject::addDrawableFromFace(osg::Geode * geode, Face if (drawable.valid()) { if (ssi.stateset) - drawable->setStateSet(ssi.stateset); + drawable->setStateSet(ssi.stateset.get()); geode->addDrawable(drawable.get()); } } diff --git a/src/osgPlugins/3ds/lib3ds/lib3ds_util.c b/src/osgPlugins/3ds/lib3ds/lib3ds_util.c index f94878041..60369a7a1 100644 --- a/src/osgPlugins/3ds/lib3ds/lib3ds_util.c +++ b/src/osgPlugins/3ds/lib3ds/lib3ds_util.c @@ -41,7 +41,16 @@ void lib3ds_util_reserve_array(void ***ptr, int *n, int *size, int new_size, int (*ptr)[i] = 0; } } - *ptr = (void**)realloc(*ptr, sizeof(void*) * new_size); + if (*ptr || new_size != 0) + { + // As this reserve_array function is also used for cleanup + // by passing in zero as new_size, we only want to call realloc + // when new_size non zero OR there is an existing array allocated. + // This is because realloc(NULL, 0) actually allocates + // storage and thus causes a memory leak when this function + // is used to clear an array that was never allocated. + *ptr = (void**)realloc(*ptr, sizeof(void*) * new_size); + } *size = new_size; if (*n > new_size) { *n = new_size;