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

[Even Rouault]

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().