[GRASS-dev] [GRASS GIS] #413: doxygen comments in get_row.c are wrong?

#413: doxygen comments in get_row.c are wrong?
-------------------------+--------------------------------------------------
Reporter: karme | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: default | Version: unspecified
Keywords: | Platform: Unspecified
      Cpu: Unspecified |
-------------------------+--------------------------------------------------
Either the code in get_row.c is wrong or the the doxygen strings.
G_get_raster_row does not return 0 if the row is outside the window.

Attached a patch trying to fix this. Note: this was a quick hack I tested
while trying to speed up d.rast => only supplied to explain the problem.

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

#413: doxygen comments in get_row.c are wrong?
-------------------------+--------------------------------------------------
Reporter: karme | Owner: grass-dev@…
     Type: defect | Status: new
Priority: major | Milestone: 6.4.3
Component: Default | Version: svn-releasebranch64
Keywords: | Platform: Unspecified
      Cpu: Unspecified |
-------------------------+--------------------------------------------------
Changes (by neteler):

  * version: unspecified => svn-releasebranch64
  * milestone: 6.4.0 => 6.4.3

Comment:

Devs: any opinion?

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

#413: doxygen comments in get_row.c are wrong?
-------------------------+--------------------------------------------------
Reporter: karme | Owner: grass-dev@…
     Type: defect | Status: new
Priority: major | Milestone: 6.4.3
Component: Default | Version: svn-releasebranch64
Keywords: | Platform: Unspecified
      Cpu: Unspecified |
-------------------------+--------------------------------------------------

Comment(by glynn):

Replying to [ticket:413 karme]:
> Either the code in get_row.c is wrong or the the doxygen strings.

The doxygen strings are wrong.

> G_get_raster_row does not return 0 if the row is outside the window.

There's no reason for higher-level code to care about what is essentially
an implementation detail. It shouldn't matter whether the row is all-nulls
because it's outside the array or because the stored data happens to be
all-nulls.

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

#413: doxygen comments in get_row.c are wrong?
-----------------------------------------+----------------------------------
Reporter: karme | Owner: grass-dev@…
     Type: defect | Status: new
Priority: major | Milestone: 6.4.4
Component: LibGIS | Version: svn-releasebranch64
Keywords: G_get_raster_row(), doxygen | Platform: All
      Cpu: All |
-----------------------------------------+----------------------------------
Changes (by hamish):

  * keywords: => G_get_raster_row(), doxygen
  * platform: Unspecified => All
  * component: Default => LibGIS
  * cpu: Unspecified => All
  * milestone: 6.4.3 => 6.4.4

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

#413: doxygen comments in get_row.c are wrong?
-----------------------------------------+----------------------------------
Reporter: karme | Owner: grass-dev@…
     Type: defect | Status: new
Priority: major | Milestone: 6.4.4
Component: LibGIS | Version: svn-releasebranch64
Keywords: G_get_raster_row(), doxygen | Platform: All
      Cpu: All |
-----------------------------------------+----------------------------------

Comment(by mlennert):

Replying to [comment:2 glynn]:
> Replying to [ticket:413 karme]:
> > Either the code in get_row.c is wrong or the the doxygen strings.
>
> The doxygen strings are wrong.
>
> > G_get_raster_row does not return 0 if the row is outside the window.
>
> There's no reason for higher-level code to care about what is
essentially an implementation detail. It shouldn't matter whether the row
is all-nulls because it's outside the array or because the stored data
happens to be all-nulls.

IIUC, return values are either 1 or -1, so just erasing the line in the
comments mentioning "return 0" should be enough ?

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

#413: doxygen comments in get_row.c are wrong?
-----------------------------------------+----------------------------------
Reporter: karme | Owner: grass-dev@…
     Type: defect | Status: new
Priority: major | Milestone: 6.4.4
Component: LibGIS | Version: svn-releasebranch64
Keywords: G_get_raster_row(), doxygen | Platform: All
      Cpu: All |
-----------------------------------------+----------------------------------

Comment(by glynn):

Replying to [comment:4 mlennert]:

> IIUC, return values are either 1 or -1, so just erasing the line in the
comments mentioning "return 0" should be enough ?

Right.

The internal functions compute_window_row() and get_map_row_nomask()
return 0 if the row is outside of the map's data, but that detail doesn't
propagate up to any of the documented public functions.

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

#413: doxygen comments in get_row.c are wrong?
---------------------+------------------------------------------------------
  Reporter: karme | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: major | Milestone: 6.4.4
Component: LibGIS | Version: svn-releasebranch64
Resolution: fixed | Keywords: G_get_raster_row(), doxygen
  Platform: All | Cpu: All
---------------------+------------------------------------------------------
Changes (by mlennert):

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

Comment:

Replying to [comment:5 glynn]:
> Replying to [comment:4 mlennert]:
>
> > IIUC, return values are either 1 or -1, so just erasing the line in
the comments mentioning "return 0" should be enough ?
>
> Right.
>
> The internal functions compute_window_row() and get_map_row_nomask()
return 0 if the row is outside of the map's data, but that detail doesn't
propagate up to any of the documented public functions.

Correction of doxygen comment committed to devel6 (r60143) and rel64
(r60144).

Closing the bug.

Moritz

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