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

[Glynn Clements]

[CC to GRASS5]

Helena wrote:

> > The bug is in r.slope.aspect, not the libraries.
> 
> you are right
> 
> > 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);
> 
> the above scik1*pcurv is a leftover from integer GRASS (that is how curvatures
> were handled in s.surf.tps because they are usually between -1,0,1).
> I believe that it can be removed (same for tcurv)

I'm not so sure; a quick test using elevation.dem from spearfish
indicates that the resulting values are likely to be much smaller than
one (e.g. +/- 0.03 for elevation.dem) so, without the scale factor, a
CELL map would be all zeroes.

This also applies to the dx/dy/dxx/dxy/dyy maps; they are all zero if
you use "prec=int". Only slope and aspect produce reasonable CELL
maps.

> I am just curious why is there CELL type, when would it be CELL, shouldn't it be
> always DCELL? (I hope that this question isn't too stupid)

The output type is an option:

    parm.out_precision = G_define_option() ;
    parm.out_precision->key        = "prec";
    parm.out_precision->type       = TYPE_STRING ;
    parm.out_precision->required   = NO ;
    parm.out_precision->answer     = "float";
    parm.out_precision->options  = "default,double,float,int";
    parm.out_precision->description= "type of output aspect and slope maps" ;

Contrary to the description, this is used for all output maps.

So, should the scik1 scalar be used for dx/dy/dxx/dxy/dyy, or should
maps other than slope/aspect be forced to use FCELL/DCELL?

> > 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).

I'm presuming that the code should match the various derivative maps,
so I'll fix it on that basis.

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