[GRASS-dev] GDAL ticket 2537: lib/gis/range.c and lib/gis/find_file.c issues

[Glynn Clements]

Markus Neteler wrote:

> >> Please take a look at
> >> http://trac.osgeo.org/gdal/ticket/2537
> >
> > Someone else needs to be reminded that the solution to memory leaks in
> > GRASS libraries is not to use those libraries from persistent
> > applications. The GRASS libraries exist for GRASS, and are designed
> > for GRASS; whether or not they are useful for anything else isn't
> > really an issue.
> >
> > OTOH, I've committed a fix for the descriptor leak in
> > lib/proj/datum.c; that actually matters.
> 
> Here the followup:
> 
> Comment (by rouault):
>  Markus, thanks, but as far as I can see, only the leak of FILE* in
>  lib/proj/datum.c have been applied. There's still a leak of FILE* in
>  lib/proj/ellipse.c that was addressed by my patch.

Okay, I've fixed that one also.

As for the memory leaks, some of them can be fixed by just using
sprintf() instead of G_asprintf(). Some of them involve data which is
known never to leave the function in question, so freeing the data
would be safe.

Others, e.g. the G_reset_mapsets() change, are unsafe, as other code
could be holding onto the pointers which are being freed. Changing
this would require adding G_store() calls and relying upon the caller
to free the data (or just allow it to leak). I think that the env.c
change falls into this category also.

A more problematic case is freeing the opt_in[i] in
lib/proj/get_proj.c. That array is passed to pj_init(). Does PROJ
guarantee that it won't hold onto those pointers? Unless it's actually
specified, even if the current version doesn't, some future version
might.

If in doubt, it's safer to just not free the data. Memory leaks at
this level don't matter to GRASS. They might matter to people trying
to build persistent applications on top of GRASS libraries, but I'm
not sure that should be our problem. Especially when they will have
plenty of other problems, e.g. G_fatal_error().

-- 
Glynn Clements <glynn at gclements.plus.com>