[GRASS-dev] [GRASS GIS] #1173: v.surf.rst: bug in handling Output deviations vector point file

GDAL · September 21, 2010, 4:03pm

#1173: v.surf.rst: bug in handling Output deviations vector point file
---------------------------------+------------------------------------------
Reporter: mmetz | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 6.4.1
Component: Vector | Version: svn-releasebranch64
Keywords: vector, rst library | Platform: All
Cpu: All |
---------------------------------+------------------------------------------
The output deviations vector file name is passed as char* to
IL_init_params_2d() in the rst library which is a bug:

{{{
main.c: In function ‘main’:
main.c:686: warning: passing argument 41 of ‘IL_init_params_2d’ from
incompatible pointer type
/home/markus/src/grass-6.4.svn/dist.i686-pc-linux-
gnu/include/grass/interpf.h:82: note: expected ‘struct FILE *’ but
argument is of type ‘char *’
}}}

Applies to all branches.

A possible solution could be to change IL_init_params_2d and make argument
number 41 a char* because the vector deviation points file has already
been created by v.surf.rst when IL_init_params_2d is called, so FILE * is
not an option.

Markus M

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/1173>
GRASS GIS <http://grass.osgeo.org>

GDAL · September 22, 2010, 10:25am

#1173: v.surf.rst: bug in handling Output deviations vector point file
---------------------------------+------------------------------------------
Reporter: mmetz | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 6.4.1
Component: Vector | Version: svn-releasebranch64
Keywords: vector, rst library | Platform: All
Cpu: All |
---------------------------------+------------------------------------------

Comment(by glynn):

Replying to [ticket:1173 mmetz]:

> A possible solution could be to change IL_init_params_2d

If you're going to do that, can you finish the job and create a sane
interface for initialisation. A single function with 40-odd parameters
isn't particularly convenient.

I've been getting this warning:
{{{
main.c:596: warning: passing argument 41 of 'IL_init_params_2d' from
incompatible pointer type
}}}
for as long as I can remember. A while back, I went through the code
fixing warnings. When I got to that one, it was a case of "argument 41 is
... 1, 2, 3, 4, ... ah, forget it" and I moved on to the next one.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/1173#comment:1>
GRASS GIS <http://grass.osgeo.org>

GDAL · September 22, 2010, 12:07pm

#1173: v.surf.rst: bug in handling Output deviations vector point file
---------------------------------+------------------------------------------
Reporter: mmetz | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 6.4.1
Component: Vector | Version: svn-releasebranch64
Keywords: vector, rst library | Platform: All
Cpu: All |
---------------------------------+------------------------------------------

Comment(by mmetz):

Replying to [comment:1 glynn]:
> Replying to [ticket:1173 mmetz]:
>
> > A possible solution could be to change IL_init_params_2d
>
> If you're going to do that, can you finish the job and create a sane
interface for initialisation. A single function with 40-odd parameters
isn't particularly convenient.

Not likely that I'm going to do that, I don't really understand the RST
library, currently don't have time to dive into it, and these 40-odd
function arguments (not only for IL_init_params_2d) are scaring me off...
I have created the ticket as a general reminder in the hope that it will
be fixed at some stage (hopefully for grass7.0 stable). Or this deviation
option gets kicked out if unused.

Markus M

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/1173#comment:2>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 2, 2013, 3:27am

#1173: v.surf.rst: bug in handling Output deviations vector point file
---------------------------------+------------------------------------------
Reporter: mmetz | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 6.4.1
Component: Vector | Version: svn-releasebranch64
Keywords: vector, rst library | Platform: All
Cpu: All |
---------------------------------+------------------------------------------

Comment(by helena):

is this still an issue? It works in 6.4.3RC3 and we use the devi option a
lot to get the stats (through v.univar) and spatial distribution of
smoothing (by interpolating the deviations into raster or displaying the
points colored by deviation),
so please do not kick out the deviation option.

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

GDAL · May 2, 2013, 4:37am

#1173: v.surf.rst: bug in handling Output deviations vector point file
---------------------------------+------------------------------------------
Reporter: mmetz | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 6.4.1
Component: Vector | Version: svn-releasebranch64
Keywords: vector, rst library | Platform: All
Cpu: All |
---------------------------------+------------------------------------------

Comment(by hamish):

Replying to [comment:3 helena]:
> is this still an issue?

yes, still get the compiler warning:
{{{
main.c:706: warning: passing argument 41 of 'IL_init_params_2d'
from incompatible pointer type
}}}

which is:
{{{
     IL_init_params_2d(&params, NULL, 1, 1, zmult, KMIN, KMAX, maskmap,
n_rows,
                       n_cols, az, adx, ady, adxx, adyy, adxy, fi, KMAX2,
                       SCIK1, SCIK2, SCIK3, rsm, elev, slope, aspect,
pcurv,
                       tcurv, mcurv, dmin, x_orig, y_orig, deriv, theta,
                       scalex, Tmp_fd_z, Tmp_fd_dx, Tmp_fd_dy, Tmp_fd_xx,
                       Tmp_fd_yy, Tmp_fd_xy, devi, inhead.time, cv,
                       parm.wheresql->answer);
}}}

as mentioned by mmetz, opt # 41 is `devi`, which is char*, later set to
devi = parm.devi->answer;

which is the string-name of the deviations vector points file.

lib/rst/interp_float/interpf.h defines the parameter as:
{{{
FILE *fddevi; /* pointer to deviations file */
}}}

and the only reason it doesn't end in tears is that
lib/rst/interp_float/point2d.c is just testing to see if it is set or not,
not trying to write anything to the fp:
{{{
if (params->fddevi != NULL)
...
}}}

v.surf.rst's main.c has a global called `FILE *fddevi`, which is not used
within the module itself.

char*devi is used to open a new points map,
Vect_open_new(&Map2, devi, 1);

and so perhaps Map2->hist_fp (FILE*) from the Map_info struct could be
passed, but it seems non-ideal. There is also Map_info->dig_fp to look at
(type GVFILE).

and/or do an audit and change the IL_init_params_2d() fn prototype and
hope it doesn't break anyone's personal addon module.

Hamish

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

GDAL · May 2, 2013, 4:39am

#1173: v.surf.rst: bug in handling Output deviations vector point file
---------------------------------+------------------------------------------
Reporter: mmetz | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 6.4.4
Component: Vector | Version: svn-releasebranch64
Keywords: vector, rst library | Platform: All
Cpu: All |
---------------------------------+------------------------------------------
Changes (by hamish):

* milestone: 6.4.1 => 6.4.4

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

GDAL · April 4, 2015, 1:01am

#1173: v.surf.rst: bug in handling Output deviations vector point file
---------------------------------+------------------------------------------
Reporter: mmetz | Owner: grass-dev@…
Type: defect | Status: new
Priority: trivial | Milestone: 7.1.0
Component: Vector | Version: svn-releasebranch64
Keywords: vector, rst library | Platform: All
Cpu: All |
---------------------------------+------------------------------------------
Changes (by wenzeslaus):

* priority: normal => trivial
* milestone: 6.4.4 => 7.1.0

Comment:

According to analysis in this ticket, the issue is small because the
pointer is used just to test against NULL and not to actual access to the
memory (which would be disaster in this case). It should be fixed anyway,
obviously.

One possible way to go is to start by putting the code from
`IL_init_params_2d()` to where the function is used. The function anyway
just sets the attributes of a structure, setting them directly can be
actually much more readable.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1173#comment:6>
GRASS GIS <http://grass.osgeo.org>