MapServer / MapServer-import

3 stars 2 forks source link

swig interface files contain preprocessor directives that will be ignored by swig #1890

Open tbonfort opened 12 years ago

tbonfort commented 12 years ago

Reporter: unicoletti Date: 2006/09/01 - 09:30

While doing the usual house keeping before 4.10 gets out I have looked
at the source of the swig interface files and found that they contain
preprocessor defines (#ifdef USE_GEOS for example).
Most of the mapscript laguages will ignore these defines because swig
is not passed the list of options built by configure. Java mapscript
used to pass the list to swig, but this feature was removed a long
time ago because Sean opted for removing the preprocessor defines from
the swig interface files to keep them tidier.
C# mapscript at this point is the only laguage still passing defines to swig.
Perl, Ruby and Python invoke swig without passing the defines; also,
in the case of Perl and Python swig must be run manually by the user
(by typing the right command in the shell).

Now since most mapscript will ignore those preprocessor defines we are
facing a potentially critical issue and, in the case of GEOS,
mapscript won't use some GEOS functions.
Other possible errors could be memory errors due to the use of non
initialized variables (this is the case of POINT_Z_M).

I have reviewed the swig interface files and found these possibly
harmful defines:

swiginc/image.i:#if GD2_VERS > 1

Not sure about this one, maybe it is right. Dunno where GD2_VERS is defined.

swiginc/image.i:#ifdef USE_GD_GIF
swiginc/image.i:#ifdef USE_GD_PNG
swiginc/image.i:#ifdef USE_GD_JPEG
swiginc/image.i:#ifdef USE_GD_WBMP

These were added by Sean in July 04 and should be removed, I guess.
They only affect TCL.

swiginc/point.i:#ifdef USE_POINT_Z_M
swiginc/point.i:#ifdef USE_POINT_Z_M
swiginc/point.i:#ifdef USE_POINT_Z_M
swiginc/point.i:#ifdef USE_POINT_Z_M
swiginc/point.i:#ifdef USE_POINT_Z_M
swiginc/point.i:#ifdef USE_POINT_Z_M

These are *rarely* used and they are not a real problem, at least for
4.10. If used they crash Java mapscript for sure.

swiginc/shape.i:#ifdef USE_GEOS
swiginc/shape.i:#ifdef USE_GEOS

These were added by sdlime on his lates rework of geos code this june.
They should be moved out of the swig interface file into the c code of mapserver
tbonfort commented 12 years ago

Author: unicoletti Date: 2006/09/01 - 09:34

Tamas,
I have reviewed all the .h files included in mapscript.i and I have
found that there are only a few unsafe usages of defines (and they
refer to the usual suspects, that is z/m members for points and geos).

In mapprimitive.h

line 74 (Definitely UNSAFE!!)
#ifdef USE_POINT_Z_M
 double z;
 double m;
#endif

line 100 (UNSAFE, but fixable):
#ifdef USE_GEOS
 void *geometry;
#endif

Other suspicious usages could be in mapows.h, but at this time I am
not sure whether they are actually posing a problem.

I don't see an easy solution to the z/m defines, but I attach a patch
that removes the need for #ifdef USE_GEOS in mapprimitive.h. I have
made a couple of tests and it seems ok to me, but I want your valuable
feedback and Steve's approval before I commit it.

Umberto

On 8/31/06, Tamas Szekeres <szekerest@gmail.com> wrote:
> Hi Umberto,
>
> How your proposal will prevent from the need of settings the defines to SWIG?
> We have ifdefs in some of the headers (eg. in map.h) will continue to
> affect the interface generation.
>
> Tamas
>
tbonfort commented 12 years ago

Author: unicoletti Date: 2006/09/01 - 20:18

The patch has been applied to cvs HEAD.
I have run my usual batch of tests and they all pass.

I'm leaving POINT_Z_M alone for 4.10.
tbonfort commented 12 years ago

Author: unicoletti Date: 2006/09/01 - 20:20

Steve, assigning this bug to myself.
tbonfort commented 12 years ago

Author: unicoletti Date: 2011/03/19 - 18:37 old, closing as fixed. the z/m problem never came up again anyway