[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