but the process continues to completion and I get a resulting map which looks
ok, but is subtly wrong.
The read error should trigger a G_fatal_error().
Agreed. The question is: why does libgis leave this to individual
modules rather than doing it itself? Are there really modules which
might want to do something other than call G_fatal_error() in this
case?
FWIW, it isn't just r.series; the following all call G_get_raster_row
(and similar) without checking the return value:
> The read error should trigger a G_fatal_error().
Agreed. The question is: why does libgis leave this to individual
modules rather than doing it itself? Are there really modules which
might want to do something other than call G_fatal_error() in this
case?
I doubt it.
Is there a possibility of many orphaned .tmp files if the module exits
during a read while an output map open? i.e. how important is it to exit
with clean-up at any given point of a typical raster [read; do stuff;
write] operation?
> > The read error should trigger a G_fatal_error().
>
> Agreed. The question is: why does libgis leave this to individual
> modules rather than doing it itself? Are there really modules which
> might want to do something other than call G_fatal_error() in this
> case?
I doubt it.
Is there a possibility of many orphaned .tmp files if the module exits
during a read while an output map open?
Yes.
i.e. how important is it to exit
with clean-up at any given point of a typical raster [read; do stuff;
write] operation?
Almost nothing does this at present. Even modules which check the
return value from G_get_raster_row() etc usually just call
G_fatal_error() if it fails.
There are precisely 20 files in 15 modules which call G_unopen_cell(),
while there are rather more than that which create new raster maps.
The remainder simply don't bother to clean up upon errors (including
r.example).
I suggest that:
a) G_get_raster_row() etc should automatically call G_fatal_error() if
an error occurs, with a function to disable this behaviour for modules
which really need to do their own error handling.
b) It should be the library's responsibility to clean up incomplete
maps upon exit.
Having to manually check for error conditions and perform clean-up
whenever an error occurs is enough of a nuisance that it will
frequently be omitted (or, if error handling is added, those code
paths will seldom get tested and thus will frequently contain bugs).
a) G_get_raster_row() etc should automatically call G_fatal_error() if
an error occurs, with a function to disable this behaviour for modules
which really need to do their own error handling.
ok
b) It should be the library's responsibility to clean up incomplete
maps upon exit.
Having to manually check for error conditions and perform clean-up
whenever an error occurs is enough of a nuisance that it will
frequently be omitted (or, if error handling is added, those code
paths will seldom get tested and thus will frequently contain bugs).
does that mean you have to start sending the file descriptors, etc,
along to the lib fns? While one read error probably means many, waiting
for cleanup-on-GRASS-exit seems less messy somehow.
Should G_get_raster_row() do system("$GISBASE/etc/clean_temp") before
calling G_fatal_error() ?
> a) G_get_raster_row() etc should automatically call G_fatal_error() if
> an error occurs, with a function to disable this behaviour for modules
> which really need to do their own error handling.
ok
> b) It should be the library's responsibility to clean up incomplete
> maps upon exit.
>
> Having to manually check for error conditions and perform clean-up
> whenever an error occurs is enough of a nuisance that it will
> frequently be omitted (or, if error handling is added, those code
> paths will seldom get tested and thus will frequently contain bugs).
does that mean you have to start sending the file descriptors, etc,
along to the lib fns? While one read error probably means many, waiting
for cleanup-on-GRASS-exit seems less messy somehow.
Should G_get_raster_row() do system("$GISBASE/etc/clean_temp") before
calling G_fatal_error() ?
My comment was limited to cleaning up the temporary cell/fcell file
created when writing new raster maps. Essentially, G_open_raster_new()
etc would need to use atexit() to automatically "unopen" any
incomplete maps.
Applications would be responsible for cleaning up their own temporary
files.