Mismatched free() / delete / delete []

[GoogleCodeExporter]

Detected via valgrind:

==16377== Thread 15:
==16377== Mismatched free() / delete / delete []
==16377==    at 0x4C26DCF: operator delete(void*) (vg_replace_malloc.c:387)
==16377==    by 0x6C87FA6: mp4v2::impl::MP4RtpHintTrack::ReadHint(unsigned int, 
unsigned short*) (rtphint.cpp:127)
==16377==    by 0x6C51284: mp4v2::impl::MP4File::ReadRtpHint(unsigned int, 
unsigned int, unsigned short*) (mp4file.cpp:3849)
==16377==    by 0x6C3FF8C: MP4ReadRtpHint (mp4.cpp:3556)
==16377==    by 0x46F5D6: MP4RtpTrack::Read(MP4RtpTrack::Listener*) 
(mp4streamer.cpp:515)
==16377==    by 0x46EFE3: MP4Streamer::PlayLoop() (mp4streamer.cpp:304)
==16377==    by 0x46EDBE: MP4Streamer::play(void*) (mp4streamer.cpp:218)
==16377==    by 0x5D6A9C9: start_thread (pthread_create.c:300)
==16377==    by 0x861270C: clone (clone.S:112)
==16377==  Address 0xa4c8500 is 0 bytes inside a block of size 784 alloc'd
==16377==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)
==16377==    by 0x6C3C7F4: mp4v2::impl::MP4Malloc(unsigned long) (mp4util.h:56)
==16377==    by 0x6C73DA0: mp4v2::impl::MP4Track::ReadSample(unsigned int, 
unsigned char**, unsigned int*, unsigned long*, unsigned long*, unsigned long*, 
bool*, bool*, unsigned int*) (mp4track.cpp:320)
==16377==    by 0x6C88000: mp4v2::impl::MP4RtpHintTrack::ReadHint(unsigned int, 
unsigned short*) (rtphint.cpp:136)
==16377==    by 0x6C51284: mp4v2::impl::MP4File::ReadRtpHint(unsigned int, 
unsigned int, unsigned short*) (mp4file.cpp:3849)
==16377==    by 0x6C3FF8C: MP4ReadRtpHint (mp4.cpp:3556)
==16377==    by 0x46F5D6: MP4RtpTrack::Read(MP4RtpTrack::Listener*) 
(mp4streamer.cpp:515)
==16377==    by 0x46EFE3: MP4Streamer::PlayLoop() (mp4streamer.cpp:304)
==16377==    by 0x46EDBE: MP4Streamer::play(void*) (mp4streamer.cpp:218)
==16377==    by 0x5D6A9C9: start_thread (pthread_create.c:300)
==16377==    by 0x861270C: clone (clone.S:112)

change rtphint.cpp:127
    delete m_pReadHintSample;

by
    free(m_pReadHintSample);

Original issue reported on code.google.com by sergio.g...@gmail.com on 12 May 2011 at 2:08

[GoogleCodeExporter]

Looks right to me.  Do you have a file that triggers this bug?

Original comment by dbyr...@gmail.com on 12 May 2011 at 2:55

[GoogleCodeExporter]

The m_pReadHintSample is created in rtphint.cpp:136 by calling the ReadSample

    // read the desired hint sample into memory
    ReadSample(
        hintSampleId,
        &m_pReadHintSample,
        &m_readHintSampleSize,
        &m_readHintTimestamp);

And in the read sample function it is allocated with malloc in mp4track.cpp:320

void MP4Track::ReadSample(
    MP4SampleId   sampleId,
    uint8_t**     ppBytes,
    uint32_t*     pNumBytes,
    MP4Timestamp* pStartTime,
    MP4Duration*  pDuration,
    MP4Duration*  pRenderingOffset,
    bool*         pIsSyncSample,
    bool*         hasDependencyFlags, 
    uint32_t*     dependencyFlags )
[...]

    if (*ppBytes == NULL) {
        *ppBytes = (uint8_t*)MP4Malloc(*pNumBytes);
        bufferMalloc = true;
    }

Then it is destroyed with delete instead of free in rtphint.cpp:127
    delete m_pReadHintSample;

Don't know if it is important, but I was getting some random seg faults in 
mp4v2 functions in my application and valgrind only complained about this. I 
believe it is good to fix it anyway.

Original comment by sergio.g...@gmail.com on 12 May 2011 at 3:10

[GoogleCodeExporter]

I totally agree with your analysis.  I just wanted to see it crash myself 
before I committed a fix.

Original comment by dbyr...@gmail.com on 12 May 2011 at 3:37

[GoogleCodeExporter]

The crash was on another point of the library, but probably related to a memory 
corruption elsewhere. I will post if I found anyway of replicate the bug.

Original comment by sergio.g...@gmail.com on 12 May 2011 at 5:23

[GoogleCodeExporter]

Agree; stuff allocated with MP4Malloc should always be free'd with MP4Free.  
This change should be in r464.  Thanks for the heads up--if you run across 
anything else in valgrind, definitely let us know so we can take a look.  
Thanks!

Original comment by kid...@gmail.com on 14 May 2011 at 11:42

• Changed state: Fixed