Opened 10 years ago

Last modified 15 months ago

#1173 new defect

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

Reported by: mmetz Owned by: grass-dev@…
Priority: trivial Milestone: 7.2.4
Component: Vector Version: svn-releasebranch64
Keywords: vector, rst library Cc:
CPU: All Platform: All

Description

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

Change History (13)

comment:1 in reply to:  description ; Changed 10 years ago by 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.

comment:2 in reply to:  1 Changed 10 years ago by 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

comment:3 Changed 7 years ago 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.

comment:4 in reply to:  3 Changed 7 years ago by 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

comment:5 Changed 7 years ago by hamish

Milestone: 6.4.16.4.4

comment:6 Changed 5 years ago by wenzeslaus

Milestone: 6.4.47.1.0
Priority: normaltrivial

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.

comment:7 Changed 4 years ago by neteler

Milestone: 7.1.07.2.0

Milestone renamed

comment:8 Changed 4 years ago by neteler

Milestone: 7.2.07.2.1

Ticket retargeted after milestone closed

comment:9 Changed 3 years ago by martinl

Milestone: 7.2.17.2.2

comment:10 Changed 3 years ago by neteler

Milestone: 7.2.27.2.3

Ticket retargeted after milestone closed

comment:11 Changed 2 years ago by martinl

Milestone: 7.2.3

Ticket retargeted after milestone closed

comment:12 Changed 2 years ago by martinl

Milestone: 7.2.4

comment:13 Changed 15 months ago by martinl

Still relevant?

Note: See TracTickets for help on using tickets.