[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