[Qgis-developer] Inconsistant use of VSIMalloc() / malloc() and VSIFree() / free()

[Etienne Tourigny]

Hi,

my opinion is that VSIMalloc/VSIFree should be used mainly by the
gdal/ogr providers, and if needed elsewhere (e.g. when calling
gdal/ogr code), but alloc/free only internally to a function/class.

Basically, following Even's recommendations.

for example, in QgsRasterDataProvider::readBlock :
  // TODO: replace VSIMalloc, it is GDAL function
  void * data = VSIMalloc( dataTypeSize( bandNo ) * width * height );
...
return data;


QgsRasterFileWriter makes much use of VSIMalloc/VSIFree but that's
probably ok, because its use seems to be limited to internal use and
passed to QgsRasterIterator, but freed locally

However, QgsRasterFileWriter::writeImageRaster uses VSIMalloc()
followed by CPLFree() - is that ok?

Even - did you try to save a raster (using new raster save as dialog)
with your modifications?


On the other hand, it could go the other way around - make sure that
all frees are using VSIFree() instead of free().

Even - what are the fundamental advantages of using VSIMalloc()
instead of malloc()?


cheers,
Etienne

On Fri, Sep 28, 2012 at 5:45 PM, Even Rouault
<even.rouault at mines-paris.org> wrote:
> Hi,
>
> I'm running QGIS master with a GDAL trunk built with a mechanism to detect
> unbalanced use of VSIMalloc() and VSIFree().
>
> To enable it, edit gdal/port/cpl_vsisimple.cpp and uncomment the //#define
> DEBUG_VSIMALLOC line at the beginning of the file. When this is enabled, the
> pointer returned by VSIMalloc() shouldn't be passed to free(). And if
> VSIFree() is provided a pointer allocated by malloc(), it will abort.
>
> This is mainly usefull to detect issues on Windows : if QGIS and GDAL are
> built with different MSVC versions, the C runtimes might not be compatible, and
> a free() done in DLL A of a pointer allocated by malloc() in a DLL B might
> crash. For example, when called from QGIS, free(VSIMalloc()) or
> VSIFree(malloc()) can result in crashes.
>
> And indeed I get crashes in QGIS when it is enabled, for example on the free()
> in qgsrasterdrawer.cpp:73
>
> I've searched in QGIS code and I can see that several classes implement void *
> XXXXX::readBlock( int bandNo, QgsRectangle  const & extent, int width, int
> height ), but they return a buffer sometimes allocated by malloc(), or
> sometimes by VSIMalloc().
>
> Some choice must be made ... My intuition would be that using regular malloc()
> / free() would be better.
>
> Basically, you would restrict the use of VSIFree() to only strings/buffers
> returned by GDAL. To avoid unnecessary changes in existing code, balanced use
> of VSIMalloc() / VSIFree() in the context of a method is OK. The real issue is
> when a method returns a void* pointer and you don't know which free function
> must be called on it.
>
> Best regards,
>
> Even
>
> PS: The same holds for CPLMalloc() / CPLFree(). In the context of the above
> analysis, they can be considered as strictly equivalent to VSIMalloc() /
> VSIFree().
> _______________________________________________
> Qgis-developer mailing list
> Qgis-developer at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/qgis-developer