Z_Realloc in p_arch

skyjake

<i>This post was originally made by <b>skyjake</b> on the dengDevs blog. It was posted under the category: Engine.</i>

There is a bunch of Z_Realloc/Z_Malloc combos used when allocating the map's data objects (lines, sectors, etc.). However, a Z_Realloc invalidates any existing pointers into the reallocated data (e.g., sectors). Planes get pointers to the sectors. Can we guarantee that a later Z_Realloc never breaks those references? Are the Z_Realloc's really needed?

DaniJ

The Z_Reallocs are there to allow multiple batches of the same map object to be created. For example, vertexes come in two batches normal and gl. I put them there as a purely temporary measure until the map loading code had been finalized, with the intention of replacing them latter (I admit I did forget about this though).

You are quite right that the realloc can potentially invalidate any existing pointers if the memory has to be moved. This will need to be fixed before Beta4 can be released.

I would not like to lose this functionality or to implement a kludge specifically for the vertexes.

We should handle this properly.

skyjake

If the reallocs are needed, then all references to the objects should be created <b>after</b> all the objects have been allocated. In this case, the plane->sector references could not be set immediately after allocation.

Another option, although a bit more risky due to its complexity, is to "mangle" the references to indices before realloc, and then "unmangle" them afterwards. This will work but requires that all the references are meticulously covered. Much more error-prone than the first option.

DaniJ

<blockquote>If the reallocs are needed, then all references to the objects should be created after all the objects have been allocated. In this case, the plane->sector references could not be set immediately after allocation.</blockquote>Yes. I did have it like that originally with all the references being set in FinalizeMapData. Problem is though that due to cross referencing in the map load code this is not really an option.

I'm thinking that mangling them would maybe be the best option. It should be possible to automate the mangle process since we know the DMT_ type of each property?

skyjake

I don't think it can be fully automated. But at least we can have a couple of helper macros, something like:
<pre>#define MANGLE_POINTER(ptr) ( *((uint*)ptr) = P_ToIndex(ptr) )
#define UNMANGLE_POINTER(ptr, dmuType) \n ( ptr = P_ToPtr(dmuType, *((uint*)ptr)) )</pre>

All the references that are set before reallocations are complete must be mangled before reallocations that may affect them.

How about storing these references that would have to mangled as indices? That would be much safer.

DaniJ

P_ToIndex won't work currently on map data objects until the global pointers have been updated.

The way it works currently is that maps are setup without touching the global object array pointers.

Its only after FinalizeMapData() has set the global pointers will calls to the DMU functions work.

Working with indices only DAM might be a good idea anyway. Then FinalizeMapData() would convert them all to pointers at the end of the read process before starting the R_Setup stuff.

skyjake

<blockquote>Working with indices only DAM might be a good idea anyway. Then FinalizeMapData() would convert them all to pointers at the end of the read process before starting the R_Setup stuff.</blockquote> Sounds robust enough to me.

DaniJ

Currently the only data type that will ever be realloc'd in practice is the vertexes. Since they don't contain any pointers I think we are OK to leave the current code alone until after Beta4.

Once Beta4 is released and I can get back to finishing the map loading code (customizable lump/map formats via DED) I'll fix this properly.