[GRASS5] r.slope.aspect crash - libes/gis/null_data.c

[Glynn Clements]

Markus Neteler wrote:

> today we realized a nice crash of r.slope.aspect:
> 
> r.slope.aspect elev pc=z.rsa.pcurf
> percent complete: Segmentation fault (core dumped)
> 
> If you try the spearfish data:
> r.slope.aspect elevation.dem pc=z.rsa.pcurf
> percent complete: Segmentation fault (core dumped)
> 
> The debugger tells us that there is something strange in
> libes/gis/null_data.c
> Program received signal SIGSEGV, Segmentation fault.
> 0x8056635 in G_set_f_null_value (fcellVals=0x0, numVals=1) at null_val.c:295
> /ssi0/ssi/neteler/cvsgrass/src/libes/gis/null_val.c:295:9157:beg:0x8056635
> (gdb) 
> 
> Unfortunately this doesn't tell me anything? Probably the NULL
> implemenation is weird in null_val.c?

The bug is in r.slope.aspect, not the libraries. The problem is with
the section of code between lines 925 and 942 in main.c:

		if (pcurv_fd > 0) {
		  if (out_type == CELL_TYPE)
		    *((CELL *) pcurv_ptr) = (CELL) (scik1 * pcurv);
		  else
		    G_set_raster_value_d(pcurv_ptr, (DCELL) pcurv, data_type);
		}
*		else
*		    G_set_null_value (pcurv_ptr, 1, data_type);
*		pcurv_ptr= G_incr_void_ptr(pcurv_ptr, G_raster_size(data_type));
                if (tcurv_fd > 0) {
                  if (out_type == CELL_TYPE)
                    *((CELL *) tcurv_ptr) = (CELL) (scik1 * tcurv);
                  else
                    G_set_raster_value_d(tcurv_ptr, (DCELL) tcurv, data_type);
                }
*               else
*                   G_set_null_value (tcurv_ptr, 1, data_type);
*               tcurv_ptr= G_incr_void_ptr(tcurv_ptr, G_raster_size(data_type));

The [pt]curv_ptr values are only valid if the corresponding
[pt]curv_fd value is also valid; otherwise, they are NULL, hence the
segfault.

Note that r.slope.aspect will also crash if you use "tc=..." without
using "pc=...".

Simply removing the "else" clauses will eliminate the crash (the
G_incr_void_ptr() calls should also be moved inside the conditional). 
But I don't know if that's the "right" fix, or whether there should be
some additional logic (i.e. whether there's some case where
G_set_null_value() is supposed to be called).

-- 
Glynn Clements <glynn.clements at virgin.net>