RFC 17: Dynamic Array Sizing

[Ned Harding]

>   layerObj **layers;
>   int numlayers; /* number of layers in mapfile */
>   int maxlayers; 

Hey Frank,

One request...  If you are going to change the meaning of the var layers
or other similar vars, can you change the name?  ppLayers or something,
frankly anything, so its made obvious to other code that links with it?

Thanks,

Ned.

-----Original Message-----
From: UMN MapServer Developers List [mailto:MAPSERVER-DEV at LISTS.UMN.EDU]
On Behalf Of Frank Warmerdam
Sent: Monday, May 15, 2006 6:54 AM
To: MAPSERVER-DEV at LISTS.UMN.EDU
Subject: Re: [UMN_MAPSERVER-DEV] RFC 17: Dynamic Array Sizing

Benedikt Rothe wrote:
> 
> Hi
> 
> Sorry for posting in this list since I'm no mapserver-developer, but I

> wonder wether RFC17 may break mapscript-applications, which hold 
> pointers to mapserver-Objects.
> 
> Example Java:
> - A Java-layerObj is created with "aMapObject.getlayer(1)". This 
> layerObj-instance holds a pointer
>   into the Layer-Array of the aMapObject.
> - After this, a new layer is added to the webobject.  The layers-Array

> in the webObject could be reallocated
>   by this action.
> - Therfore the existing pointer in the Java-layerObj could become
invalid.
> This could cause crashes.
> 
> Did I oversee something?
> Benedikt

Benedikt,

Yikes, you are quite right.  In fact whenever we actually end up
"growing"
the list of layers, classes or styles any old pointers could be
invalidated.
I don't forsee this occuring too often in the mapserver core itself
because it isn't common to keep pointers around.  Instead it is
layer/class/style indexes that are normally kept over time.

In other C/C++ contexts I would normally have my layers array as an
array of pointers to individually allocated layerObj's.  That way,
reallocating the array of pointers would not cause the existing
layerObj's to move.  But because mapserver uses a direct array of
layerObj's, reallocating inevitably means the need to move all the
existing layers in memory (and similarly for other objects).

I *suppose* we could move to individual allocations in mapserver as
well.
But I think this would cause a lot of changes in the code that would
make the effort much bigger, as well as resulting in more memory
fragmentation.

There are lots of places in the code that do stuff like this:

         lp = &(map->layers[ map->layerorder[i]]);

If we changed layers to be an array of layerObj pointers, this would
return a pointer to the layerObj pointer, instead of the pointer to the
layerObj
itself.  So quite a bit of code would need to be touched.   This change,
within the mapserver core, should be relatively routine grunt work.  I'm
not sure if it would have a significant affect on mapscript or not.  I
*think* this change would be relatively invisible to mapscript once
re-swigged and rebuilt.

I think I would be willing to try and implement individual allocation of
the layer/class/style objects if the developer would like to take this
approach.  The downside is that there is likely to be changes required
in parts of MapServer I don't normally build (like specific data
providers and specific output drivers).  There is also likely to be more
work required in the swig bindings.  Should I rewrite the RFC with this
intent?

To be clear, most of the RFC would remain the same, but something like:

   layerObj *layers;
   int numlayers; /* number of layers in mapfile */
   int maxlayers;

would become:

   layerObj **layers;
   int numlayers; /* number of layers in mapfile */
   int maxlayers;

So "layers" is a pointer to an array of layerObj pointers.  When the map
is initialized each of the initial MS_MAXLAYERS layerObj would be
individually malloced, and initialized.  when the msGrowMapLayers()
function needs to grow the layers list it would reallocate the array of
pointers larger, and allocate some new layerObj's.

Best regards,
-- 
---------------------------------------+--------------------------------
---------------------------------------+------
I set the clouds in motion - turn up   | Frank Warmerdam,
warmerdam at pobox.com
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | President OSGF,
http://osgeo.org