[GRASS-dev] [GRASS GIS] #866: Semantic of free() and G_free() for G_free_key_value()

#866: Semantic of free() and G_free() for G_free_key_value()
-------------------------+--------------------------------------------------
Reporter: mloskot | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: normal | Milestone:
Component: libgis | Version: svn-trunk
Keywords: gdal | Platform: All
      Cpu: Unspecified |
-------------------------+--------------------------------------------------
Recently, a bug has been submitted [http://trac.osgeo.org/qgis/ticket/2303
to QGIS] and forwarded [http://trac.osgeo.org/gdal/ticket/3323 to GDAL].

IMHO, the problem is in GRASS.

The function is [source:grass/trunk/lib/gis/key_value1.c?@35855#L149
G_free_key_value] not proof against nullptr and, given purpose of this
function, if pointer to key is null, it should do nothing. This is a
standard semantic of free() function in C library. Moreover, G_free
function already follows semantic of free(), so G_free_key_value() should
do it as well.

In other words, it should read as follows:

{{{
void G_free_key_value(struct Key_Value *kv)
{
    if (kv != NULL)
    {
    }
}
}}}

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/866&gt;
GRASS GIS <http://grass.osgeo.org>

#866: Semantic of free() and G_free() for G_free_key_value()
----------------------+-----------------------------------------------------
  Reporter: mloskot | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: normal | Milestone:
Component: libgis | Version: svn-trunk
Resolution: | Keywords: gdal
  Platform: All | Cpu: Unspecified
----------------------+-----------------------------------------------------
Comment (by glynn):

Replying to [ticket:866 mloskot]:

> The function is [source:grass/trunk/lib/gis/key_value1.c?@35855#L149
G_free_key_value] not proof against nullptr and, given purpose of this
function, if pointer to key is null, it should do nothing. This is a
standard semantic of free() function in C library. Moreover, G_free
function already follows semantic of free(), so G_free_key_value() should
do it as well.

Fixed in r40399 (trunk).

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/866#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

#866: Semantic of free() and G_free() for G_free_key_value()
----------------------+-----------------------------------------------------
  Reporter: mloskot | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: normal | Milestone:
Component: libgis | Version: svn-trunk
Resolution: | Keywords: gdal
  Platform: All | Cpu: Unspecified
----------------------+-----------------------------------------------------
Comment (by hamish):

Replying to [comment:1 glynn]:
> Fixed in r40399 (trunk).

shall we backport?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/866#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

#866: Semantic of free() and G_free() for G_free_key_value()
----------------------+-----------------------------------------------------
  Reporter: mloskot | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: normal | Milestone:
Component: libgis | Version: svn-trunk
Resolution: | Keywords: gdal
  Platform: All | Cpu: Unspecified
----------------------+-----------------------------------------------------
Comment (by glynn):

Replying to [comment:2 hamish]:
> shall we backport?

I see no reason not to.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/866#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

#866: Semantic of free() and G_free() for G_free_key_value()
----------------------+-----------------------------------------------------
  Reporter: mloskot | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: closed
  Priority: normal | Milestone:
Component: libgis | Version: svn-trunk
Resolution: fixed | Keywords: gdal
  Platform: All | Cpu: Unspecified
----------------------+-----------------------------------------------------
Changes (by neteler):

  * status: new => closed
  * resolution: => fixed

Comment:

Done in r40403 (6.4) and r40404 (6.5).

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/866#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

#866: Semantic of free() and G_free() for G_free_key_value()
----------------------+-----------------------------------------------------
  Reporter: mloskot | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: closed
  Priority: normal | Milestone:
Component: libgis | Version: svn-trunk
Resolution: fixed | Keywords: gdal
  Platform: All | Cpu: Unspecified
----------------------+-----------------------------------------------------
Comment (by mloskot):

Thank you guys!

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/866#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>