[GRASS-dev] Re: [GRASS GIS] #837: Memory leaks in r.example

[GRASS GIS]

#837: Memory leaks in r.example
----------------------+-----------------------------------------------------
  Reporter:  sprice   |       Owner:  grass-dev at lists.osgeo.org
      Type:  defect   |      Status:  reopened                 
  Priority:  normal   |   Milestone:  6.4.0                    
 Component:  default  |     Version:  svn-releasebranch64      
Resolution:           |    Keywords:                           
  Platform:  MacOSX   |         Cpu:  OSX/Intel                
----------------------+-----------------------------------------------------
Changes (by sprice):

  * status:  closed => reopened
  * resolution:  wontfix =>

Comment:

 Judging by the 1616 leaked nodes in get_map_row() this is on a per row
 basis. But I've never heard of a well written program containing leaks
 anyway.

 Here's your fix for the first leak:
 Change line 58 in lib/gis/gisinit.c from "G_location_path();" to
 "G_free(G_location_path());"

 The second leak:
 Add "G_free(dummy);" at line 1003 in file lib/gis/get_row.c

 The third, fourth, & last leaks (in lib/gis/opencell.c) are a bit more
 difficult to take care of all permutations:
 {{{
 --- grass-6.4.0RC5/lib/gis/opencell.c   2009-03-04 01:06:31.000000000
 -0700
 +++ grass-6.4.svn_src_snapshot_2009_12_12/lib/gis/opencell.c    2009-12-16
 00:04:45.000000000 -0700
 @@ -188,21 +188,27 @@
             G_warning(_("Unable to open raster map <%s@%s> since it is a
 reclass "
                         "of raster map <%s@%s> which does not exist"),
                       name, mapset, r_name, r_mapset);
 +        G_free(xmapset);
             return -1;
         }
         break;
      default:                   /* Error reading cellhd/reclass file */
 +            G_free(xmapset);
         return -1;
      }

      /* read the cell header */
 -    if (G_get_cellhd(r_name, r_mapset, &cellhd) < 0)
 -       return -1;
 +    if (G_get_cellhd(r_name, r_mapset, &cellhd) < 0){
 +        G_free(xmapset);
 +        return -1;
 +    }

      /* now check the type */
      MAP_TYPE = G_raster_map_type(r_name, r_mapset);
 -    if (MAP_TYPE < 0)
 -       return -1;
 +    if (MAP_TYPE < 0){
 +        G_free(xmapset);
 +        return -1;
 +    }

      if (MAP_TYPE == CELL_TYPE)
         /* set the number of bytes for CELL map */
 @@ -211,6 +217,7 @@
         if (CELL_nbytes < 1) {
             G_warning(_("Raster map <%s@%s>: format field in header file
 invalid"),
                       r_name, r_mapset);
 +        G_free(xmapset);
             return -1;
         }
      }
 @@ -220,11 +227,13 @@
                     "Found raster map <%s@%s>, should be <%s>."),
                   name, mapset, name, G__projection_name(cellhd.proj),
                   G__projection_name(G__.window.proj));
 +        G_free(xmapset);
         return -1;
      }
      if (cellhd.zone != G__.window.zone) {
         G_warning(_("Raster map <%s@%s> is in different zone (%d) than
 current region (%d)"),
                   name, mapset, cellhd.zone, G__.window.zone);
 +        G_free(xmapset);
         return -1;
      }

 @@ -232,6 +241,7 @@
      if (MAP_TYPE == CELL_TYPE && (unsigned int) CELL_nbytes >
 sizeof(CELL)) {
         G_warning(_("Raster map <%s@%s>: bytes per cell (%d) too large"),
                   name, mapset, CELL_nbytes);
 +        G_free(xmapset);
         return -1;
      }

 @@ -261,6 +271,7 @@
  #else
         G_warning(_("map <%s@%s> is a GDAL link but GRASS is compiled
 without GDAL support"),
                   r_name, r_mapset);
 +        G_free(xmapset);
         return -1;
  #endif
      }
 @@ -268,8 +279,10 @@
         /* now actually open file for reading */
         fd = G_open_old(cell_dir, r_name, r_mapset);

 -    if (fd < 0)
 -       return -1;
 +    if (fd < 0){
 +        G_free(xmapset);
 +        return -1;
 +    }

      fcb = new_fileinfo(fd);

 @@ -298,6 +311,7 @@
             fcb->name = G_store(name);
      }
      fcb->mapset = G_store(mapset);
 +    G_free(xmapset);

      /* mark no data row in memory  */
      fcb->cur_row = -1;
 @@ -977,9 +991,12 @@
         return -1;
      }
      G__file_name(path, "fcell", name, xmapset);
 -    if (access(path, 0) == 0)
 -       return 1;
 +    if (access(path, 0) == 0){
 +        G_free(xmapset);
 +        return 1;
 +    }
      G__file_name(path, "g3dcell", name, xmapset);
 +    G_free(xmapset);
      if (access(path, 0) == 0)
         return 1;

 @@ -1002,7 +1019,8 @@
  {
      char path[GPATH_MAX];
      const char *xmapset;
 -
 +    RASTER_MAP_TYPE cktype;
 +
      xmapset = G_find_cell2(name, mapset);
      if (!xmapset) {
         if (mapset && *mapset)
 @@ -1013,11 +1031,15 @@
      }
      G__file_name(path, "fcell", name, xmapset);

 -    if (access(path, 0) == 0)
 -       return G__check_fp_type(name, xmapset);
 +    if (access(path, 0) == 0) {
 +        cktype = G__check_fp_type(name, xmapset);
 +        G_free(xmapset);
 +        return cktype;
 +    }

      G__file_name(path, "g3dcell", name, xmapset);
 -
 +    G_free(xmapset);
 +
      if (access(path, 0) == 0)
         return DCELL_TYPE;
  }}}

 The next leak in lib/gis/gdal.c:
 {{{
 @@ -135,12 +139,14 @@
      RASTER_MAP_TYPE map_type;
      FILE *fp;
      struct Key_Value *key_val;
 -    const char *p;
 +    const char *p, *fc;
      DCELL null_val;

 -    if (!G_find_cell2(name, mapset))
 +    if (!(fc = G_find_cell2(name, mapset)))
         return NULL;
 -
 +
 +    G_free(fc);
 +
      map_type = G_raster_map_type(name, mapset);
      if (map_type < 0)
         return NULL;
 }}}

-- 
Ticket URL: <http://trac.osgeo.org/grass/ticket/837#comment:3>
GRASS GIS <http://grass.osgeo.org>