Opened 11 years ago

Closed 6 years ago

#413 closed defect (fixed)

doxygen comments in get_row.c are wrong?

Reported by: karme Owned by: grass-dev@…
Priority: major Milestone: 6.4.4
Component: LibGIS Version: svn-releasebranch64
Keywords: G_get_raster_row(), doxygen Cc:
CPU: All Platform: All

Description

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.

Attachments (1)

outofwindow.diff (1.3 KB) - added by karme 11 years ago.

Download all attachments as: .zip

Change History (7)

Changed 11 years ago by karme

Attachment: outofwindow.diff added

comment:1 Changed 7 years ago by neteler

Milestone: 6.4.06.4.3
Version: unspecifiedsvn-releasebranch64

Devs: any opinion?

comment:2 in reply to:  description ; Changed 7 years ago by glynn

Replying to 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.

comment:3 Changed 7 years ago by hamish

Component: DefaultLibGIS
CPU: UnspecifiedAll
Keywords: G_get_raster_row() doxygen added
Milestone: 6.4.36.4.4
Platform: UnspecifiedAll

comment:4 in reply to:  2 ; Changed 6 years ago by mlennert

Replying to glynn:

Replying to 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 ?

comment:5 in reply to:  4 ; Changed 6 years ago by glynn

Replying to 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.

comment:6 in reply to:  5 Changed 6 years ago by mlennert

Resolution: fixed
Status: newclosed

Replying to glynn:

Replying to 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

Note: See TracTickets for help on using tickets.