Tim,
> I didn't think setting a variable to
> NULL would be a problem but I can go through all the functions that just
> return locators and make sure that you don't need to intiialise variables
> on entry.
Another problem is what happens after an error. In this case it is
possible for an attempt to be made to free an HDSLoc structure pointed to
by a pointer which has never been initialised. E.g. let's say an HDSLoc
pointer initially has some garbage value such as 0x80c, and an error
occurs before the point at which this pointer is assigned a usable value.
So execution arrives at the point where the HDSLoc structure would be
freed if no error had occurred. Because of the starlink error strategy,
such a clean-up function attempts to execute even though an error has
occurred, and so tries to free the pointer 0x80c, with a consequent
segvio.
The question is how to avoid this. One answer is to depend on the caller
initialising the pointer to NULL before any error can possibly occur, and
then make HDS check for NULL values before freeing the pointer. This is
what I understand you do in HDS at the moment, but it seems a high risk
approach to me since it relies on the application code being written
correctly, and it's all too easy to forget to do this sort of
initialisation when you are writing code.
An alternative AST-like approach would be to include extra info in
the HDSLoc structure which is used to signal that the structure has
been initialised by HDS. An AST memory block has extra information giving
the size of the allocated memory block, and a magic number which is formed
by combining the address and size of the memory block. The comments from
ast/memory.c say:
* - The test performed by this function is not 100% secure as the
* "magic" value could occur by accident (although this is
* unlikely). It is mainly intended to provide security against
* programming errors, including accidental corruption of the
* memory header and attempts to allocate the same region of memory
* more than once.
What happens is that the function which imports an HDSLoc pointer can then
check this extra information for validity, skipping sensitive operations
such as "free" and reporting an error (if no error has already occurred) if
it is invalid. This guards against inappropriate pointers being supplied
by accident, and also prevents a segvio if an attempt is made to free an
uninitialied structure.
Adopting an approach such as this would do away with the dangers of
relying on application code being written correctly. It would involve:
1) adding a couple of extra int components ("size" and "magic") to the
HDSLoc structure,
2) Modifying the function which allocates memory for an HDSLoc to assign
suitable values to these new components.
3) Writing a function which checks the validity of the "size" and "magic"
values in an HDSLoc, reporting an error if they are invalid. This function
should not check the inherited status on entry, but should only report an
error if no error status existed on entry.
4) Modifying the function which imports HDSLoc structures to use the above
function to check the validity of the pointer.
5) Modify the function which frees HDSLoc structures (and any other
clean-up functions which do not check status on entry) to avoid using
the pointer if it is deemed not to be valid.
It all seems to work very smoothly in AST.
David
|