v.surf.rst: bug in handling Output deviations vector point file

[mmetz]

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

[glynn]

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

[mmetz]

Replying to glynn:

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

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

[hamish]

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

[wenzeslaus]

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.

[neteler]

Milestone renamed

[neteler]

Ticket retargeted after milestone closed

[neteler]

Ticket retargeted after milestone closed

[martinl]

Ticket retargeted after milestone closed

[martinl]

Still relevant?