Bug in imageObj's free() method

[tbonfort]

Reporter: sgillies@frii.com Date: 2003/08/04 - 19:35

Calling the free() method of an imageObj instance results in:

*** malloc[438]: Deallocation of a pointer not malloced: 0x49bef0; This could be
a double free(), or free() called with the middle of an allocated block; Try
setting environment variable MallocHelp to see tools to help debug

the free() method is

  void free() {
    msFreeImage(self);    
    free(self);
  }

I think that the free(self) statement is not right.  I've commented out 
this statement and can free() without the malloc error/warning.

Would like to fix this in the 4.0 branch as well as 4.1.

[tbonfort]

Author: sdlime Date: 2003/08/05 - 07:34

I'm not sure who wrote that method. It may have been me, but maybe not. I'm kind
of suprised msFreeImage free's the imageObj itself and not just it's resources.
I've cc'd Frank on this one since he knows the imageObj code the best so he can
confirm your fix is correct.

Steve

[tbonfort]

Author: fwarmerdam Date: 2003/08/05 - 07:53

I have checked, and it seems in rev 1.120 of maputil.c (2002/11/19) I 
change msFreeImage() to also free the imageObj itself, as well as it's
attached resources.  

Reviewing the mapserver C code it appears that this is compatible
with the usage of msFreeImage(), but clearly in 3.6.x the semantics 
were to free only the contents which is (apparently) why the free()
method on the imageObj in mapscript had to free the imageObj memory
seperately.

I appologise for changing the way msFreeImage() works without also
fixing up MapScript.  Even now I don't know why Steve feels the
msFreeImage() function shouldn't free the imageObj itself.  Why 
shouldn't this just be the opposite of msImageCreate()? 

In any event, I think Sean is right in his fix for free(), and I 
wonder if PHP MapScript might have a similar issue?  I have written
to Daniel to suggest he check. 

[tbonfort]

Author: sdlime Date: 2003/08/05 - 15:57

Thanks for checking Frank. I've never been clear on whether a function dealing 
with a structure should free just the resources or the whole thing- your logic 
clarifies that. In this case since msImageCreate forces the use of an imageObj 
pointer (rather than being able to use a static imageObj) then the opposite 
behavior of msFreeImage is perfect. Sean's fix is already in, and if Daniel can 
confirm for PHP then we can move on.

[tbonfort]

Author: sgillies@frii.com Date: 2003/08/05 - 16:54

I've committed changes to mapscript.i in the main CVS and
the branch-4-0.

[tbonfort]

Author: dmorissette Date: 2003/08/05 - 17:01

Checking the PHP code now.

[tbonfort]

Author: dmorissette Date: 2003/08/05 - 17:39

PHP MapScript was already OK.  No change required.