[Qgis-developer] Inconsistant use of VSIMalloc() / malloc() and VSIFree() / free()
Even Rouault
even.rouault at mines-paris.org
Sat Sep 29 06:59:08 PDT 2012
Le samedi 29 septembre 2012 15:24:59, Etienne Tourigny a écrit :
> 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)
This is even not needed for code that interact with GDAL/OGR. GDAL/OGR
functions that take memory buffers just works fine with whatever memory
allocator you want (malloc, new[], etc...)
The only requirement is that VSIFree( ) *must* be used to free a pointer
allocated by GDAL, to make sure that the CRT used for free() matches the CRT
used for malloc().
> , 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;
You didn't finish your sentence here. So I suppose you meant that the TODO
should be implemented by replacing VSIMalloc() by malloc() ?
>
>
> 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?
Yes, CPLFree(VSIMalloc()) is OK
The difference between VSIMalloc() and CPLMalloc() is that CPLMalloc() calls
internally VSIMalloc() and aborts if the allocation fails. Thus CPLMalloc()
never returns a NULL pointer and is only meant for very small allocations that
shouldn't fail. My personnal opinion on this is that it should be completely
avoided and replaced by VSIMalloc() with a proper check of the return value.
VSIFree() and CPLFree() are the same: in port/cpl_conv.h : "#define CPLFree
VSIFree"
>
> Even - did you try to save a raster (using new raster save as dialog)
> with your modifications?
I didn't do any modification yet. With the memory allocation debugging enabled,
QGIS just crashes when opening a raster, so I couldn't test saving ;-)
>
>
> On the other hand, it could go the other way around - make sure that
> all frees are using VSIFree() instead of free().
That's one possibility, but I was wondering if there are other raster
providers than GDAL. In that case, they are unlikely to allocate with
VSIMalloc().
>
> Even - what are the fundamental advantages of using VSIMalloc()
> instead of malloc()?
Basically none, from the application's point of view when GDAL is compiled
with default options.
For the purpose of GDAL developement, VSIMalloc() / VSIFree() are interesting,
because they can be instrumented to provide debugging and statistics
facilities. For example, if you enable both DEBUG_VSIMALLOC and
DEBUG_VSIMALLOC_STATS, you get the following report at the end of each process
using GDAL :
Current VSI memory usage : 0 bytes
Maximum VSI memory usage : 23781 bytes
Number of calls to VSIMalloc() : 770
Number of calls to VSICalloc() : 0
Number of calls to VSIRealloc() : 235
Number of calls to VSIFree() : 770
VSIMalloc + VSICalloc - VSIFree : 0
This is a cheap way of tracking most memory leaks with quite unnoticeable
slowdown. When DEBUG_VSIMALLOC is enabled, there's also a check in VSIFree()
to detect that the pointer passed is valid (i.e. allocated by VSIMalloc()) and
to detect double-frees. A kind of poor man's "valgrind --leak-check=full" to
sum it up ;-) For my developement activities in GDAL, I enable that all the
time.
Of course, other applications can benefit from that by using
VSIMalloc()/VSIFree(). But you must be very carefull to correctly pair
VSIMalloc() with VSIFree(), and malloc() with free() : any other combination
will lead to chaos.
Even
More information about the Qgis-developer
mailing list