[patch] slightly controversial refactor
Andrew Turner
andrewturner512 at googlemail.com
Sat Jan 20 20:43:53 UTC 2007
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