[RFC-24] Updated public draft

Umberto Nicoletti umberto.nicoletti at GMAIL.COM
Sun Feb 25 07:04:01 EST 2007


> > > > 3.2 Add references to the mapscript wrapper object
> > > > --------------------------------------------------------------------------
> > > > This effort can be ignored, see below.
> > > >
> > >
> > > I'm afraid we can't. There are many places where the code is like this:
> > >
> > > layer->map->layers[layersorder[i]]
> > >
> > > the parent object is accessed by the child without checking. If this
> > > happens and the parent has been gc'ed we obtain a segfault which
> > > results in a crash of the hosting language, which is exactly what we
> > > want to avoid *at any cost*.
> > >
> >
> > Well, I haven't looked over the code in this regard and needless to
> > say I'm *totally disappointed*. Using a pointer without checking
> >
> > it's value is not acceptable in most cases. Now then it's not so
> > difficult to write some test cases that will definitely fail with
> >
> > unhandled exception, like:
> >
> >      layerObj layer = new layerObj(null);
> >      layer.connection = "data.shp";
> >      layer.connectiontype = MS_CONNECTION_TYPE.MS_SHAPEFILE;
> >      layer.open();
> >
> >
> > DAMN !
> >
>
> We could find all these uncheked pointer derefences and correct them
> in the C code directly. This would have the benefit of improving the
> overall safety of the C code and (maybe) ease the implementation of
> this rfc.
> Let me check upon this.
>

Replying to myself...(split personality?)

I have checked and it seems that we can add a check at the beginning
of all functions where a layer uncorrectly dereferences the map
pointer without checking. The check will be  like this:

if (layer->map == NULL) {
   // should we raise MS_CHILDERR instead?
   msSetError(MS_MEMERR, "map is null", "");
   return MS_FAILURE;
}

It can be quite easy to find all functions that need to be patched by
using the Eclipse CDT search function (http://www.eclipse.org/cdt/).
The search results report a total of 124 references, while a good ole
grep says 111 (grep -n -e "layer->map" *.c).
Of course the same needs to be done for classes, styles, symbols, etc.

If we correct this do we still need to keep a parent reference in the
child mapscript object?

> Umberto
>



More information about the mapserver-dev mailing list