MP4Close does not null out ptr argument about mp4v2 HOT 8 CLOSED

Ugg, sorry:

f = NULL;

No dereference.  my bad.

Whoa. That's wrong in so many ways.

As stated by point #3 you desire MP4FileHandle hFile to be set to NULL after 
object is destroyed.
That is not possible as hFile must be MP4FileHandle* in order to accomplish the 
enhancement you desire.
In case it was changed, then yes a simple "*hFile = NULL" would be your 
enhancement.
Unfortunately this will break all existing MP4Close() usage.

Secondly, it is illegal to assign NULL to an object, which 'f' represents. The 
reason why you do not get a 
compiler error is because MP4File(int) constructor accepts an integer. In my 
opinion it would be best to 
change MP4File(int) --> "explicit MP4File(int)" to avoid implicit conversion of 
NULL to a new object.

"f = NULL;" does the following which I'm sure is not desired:

1. create a temporary MP4File object by using constructor MP4File(int) which 
allows NULL to be used as per 
not perfectly clear C++ standard.
2. assign the tmp object to f (this uses default object copy semantics because 
there is no custom defined 
copy constructor)
3. this behavior would be undefined because 'f' was just deleted. while it 
might not generate a coredump, 
because the memory is most likely still available, it's certainly undefined to 
use a dead object with copy 
semantics.

Small correction. It's strictly not "illegal" to assign NULL to an object. You 
need help from implicit constructors or 
overloaded assignment operators. In either case though, it's not what you 
desire to do for this issue.

In my haste, I wrote some bad code.  Sorry.  Let me explain myself better.

Here is the specific function:

    void MP4Close(MP4FileHandle hFile)
    {
        if( !MP4_IS_VALID_FILE_HANDLE( hFile ))
            return;

        MP4File& f = *(MP4File*)hFile;
        try {
            f.Close();
        }
        catch ( MP4Error* e ) {
            PRINT_ERROR( e );
            delete e;
        }

        delete &f;
    }

MP4FileHandle is simply a void*:

typedef void*       MP4FileHandle;

...that is, just a pointer.  So the fix should be:

void MP4Close(MP4FileHandle hFile)     
{
...
delete &f;     
hFile = NULL;
} 

In general, when you delete something a pointer points to (and in this case, 
hFile
points to MP4File*), you should also set it to null.  Other places in the 
library do
this consistently, so I don't know why this function wouldn't.

Also, 

"*hFile = NULL"

...would be totally wrong.  This would actually do what you seem to be 
explaining,
which would be setting a class instance to NULL, which yes, I agree is a compile
error.  Again, I apologize for not being clear and posting some bad code, but 
this is
clearly not the point I was trying to make.

But the above change absolutely does not break any MP4Close() usage; it is 
perfectly
legal to NULL out a pointer to an object that has just been deleted.

I'll omit the typedef so you can see what's happening:

void MP4Close( void* pFile ) {
    pFile = NULL; // useless as pFile is an in-parm and would do nothing to set caller's pointer
}

void MP4Close( void** ppFile ) {
   *ppFile = NULL; // sets (out) pFile to NULL
}

And as mentioned earlier, this requires an incompatible change in function 
signature.

Kona,

My apologies, the ** thing always throws me for a loop, but you're definitely 
right.
 And yes, I agree this would be a sizable change that is not worth the effort.  I'll
just null out the argument after I call MP4Close like I'm currently doing.

You can close this--thanks for the help!