[patch] slightly controversial refactor

Andrew Turner andrewturner512 at googlemail.com
Sat Jan 20 20:52:44 UTC 2007


Wait, ignore me. The patch didn't fix the memory leak, but Jeff did,
and in the same commit.

Andrew

On 20/01/07, Andrew Turner <andrewturner512 at googlemail.com> wrote:
> Jeff Mitchell wrote:
> > This is
> > used, for instance, when the mediamanager sends out a mediumChanged signal.
> > mediumChanged only gives a QString with the name of the device, so we have to
> > refresh its information ourselves.  In this instance, getDevice is called on
> > that medium, which refreshes the information from mediamanager and then sends
> > a pointer to the updated Medium (residing in m_mediumMap) out to clients.
>
> >The onus isn't really on the caller to delete the returned object
>
> So the pointer in m_mediumMap should be sent to the caller of the function?
>
> But if you examine the code (after the patch, but the problem existed before):
> <snip>
>             returnedMedium = new Medium(*it);
>             MediumIterator secIt;
>             if ( (secIt = m_mediumMap.find( name )) != m_mediumMap.end() ) {
>                 Medium* tempMedium = (*secIt);
>                 m_mediumMap.remove( name );
>                 delete tempMedium;
>             }
>             m_mediumMap[ name ] = new Medium(*it);
>             break;
> <snip>
>
> You can easily see from that code that a new Medium is created twice.
> So the returnedMedium pointer will point to a different object than
> m_mediumMap[ name ]. If you don't delete the object returnedMedium
> points to (which the caller would have to do), you have a leak,
> because that pointer is never stored - it's just returned at the end
> of the function call.
>
> Andrew
>



More information about the Amarok mailing list