Reason of REALLOC implementation ?

[jonathan at doves.demon.co.uk]

   John.Tytgat at aaug.net wrote:

> In !OsLib/Tools/support there are two files realloc.c and realloc.h which
> implement the REALLOC() function.  The file !OsLib/Tools/support/doc/realloc
> says:
> 
> --8<--
> realloc.c
> ---------
> 
>    Just a portable realloc with no bugs. It's called REALLOC, though.
> --8<--
> 
> Does someone know which bugs (and in which component) this document
> refers too ? Bugs in a cross compiled environment ? Or bugs in RISC OS SCL ?

   Well, vaguely ...

   It was SharedCLib, but I don't remember the precise problem. It may have been something to do with the fact that it used to return a non-NULL value for malloc(0).

> The strange thing is that this REALLOC() implementation is not free from
> bugs either:
> 
> --8<--
> /*realloc.c - portable realloc with no bugs!*/
> 
> [...]
> 
> #include <stdlib.h>
> #include <string.h>
> 
> #include "realloc.h"
> 
> void *REALLOC
> (
>    void   *ptr,
>    size_t  size
> )
> {
>    if (ptr != NULL && size != 0)
>    {
>       void *tmp;
> 
>       if ((tmp = malloc (size)) == NULL)
>          return NULL;
> 
>       memcpy (tmp, ptr, size);
> 
>       free (ptr);
> 
>       return tmp;
>    }
>    else if (size != 0)
>       return malloc (size);
>    else
>    {
>       /*ptr != NULL*/
>       free (ptr);
>       return NULL;
>    }
> }
> --8<--
> 
> When you make an existing malloc block bigger, the old ptr gets used
> to read the new size 'size' from and this can lead to undefined behaviour.

   But that doesn't matter, because it was only written for a specific reason where I knew the behaviour was OK. (It can read off the end of memory, but RISC OS didn't do anything bad then. This back in the days of "address exception"---no segfaults!). So the bug is that it says "portable" in the comment---it's actually RISC OS-specific.

> Also when the first malloc() call above fails, there is no free() of the
> old pointer done...

   That's actually what the spec says. To do the opposite would destroy the caller's data.

> This REALLOC() routines is used in m.h when tracing is not enabled. If there
> are no good reasons for the use of REALLOC() I would change that (back ?)
> to the standard realloc() call like it is done in the OSLibSupport code
> as well (cfr x__realloc() routine).

   Go ahead. I'm sure it would be fine.

... Jonathan