[h323plus] Double-delete Capability bug

Jan Willamowius jan at willamowius.de
Fri Jan 22 13:49:14 EST 2016

Hi Iurii,

I have added the Clone() in the CVS, but could you please check (wg.
with Valgrind) if we now have a memory leak in some cases instead of a
crash ?


Jan Willamowius, Founder of the GNU Gatekeeper Project
EMail  : jan at willamowius.de
Website: http://www.gnugk.org
Support: http://www.willamowius.com/gnugk-support.html

Relaxed Communications GmbH
Frahmredder 91
22393 Hamburg
Geschäftsführer: Jan Willamowius
HRB 125261 (Amtsgericht Hamburg)
USt-IdNr: DE286003584

Iurii Gordiienko wrote:
> Hi, 
> I have found something... For H323ExtendedVideoCapability::AddAllCapabilities function we are using H323CodecExtendedVideoCapability::AddCapability function for extCapability->AddCapability(*r). H323CodecExtendedVideoCapability::AddCapability function uses extCapabilities.Add(H323ExtendedVideoFactory::CreateInstance(cap)) function. The function H323ExtendedVideoFactory::CreateInstance(cap) will call "new" if any same "cap" does not is exists for now< for other case it will return the pointer to current actual Capability. But H323CodecExtendedVideoCapability::extCapabilities will delete all items in time of execution  extCapabilities.RemoveAll() function from H323CodecExtendedVideoCapability::~H323CodecExtendedVideoCapability(). In result we have "double delete" and undefined behavior (crash for worst case). 
> -------------------------------- 
> void H323ExtendedVideoCapability::AddAllCapabilities(       H323Capabilities & basecapabilities, PINDEX descriptorNum,PINDEX simultaneous) {   H323ExtendedVideoFactory::KeyList_T extCaps = H323ExtendedVideoFactory::GetKeyList();   if (extCaps.size() > 0) {     H323CodecExtendedVideoCapability * capability = new H323CodecExtendedVideoCapability();     H323ExtendedVideoFactory::KeyList_T::const_iterator r;         PINDEX num = P_MAX_INDEX;         for (r = extCaps.begin(); r != extCaps.end(); ++r) {            H323CodecExtendedVideoCapability * extCapability = (H323CodecExtendedVideoCapability *)capability->Clone();            extCapability->AddCapability(*r);            num = basecapabilities.SetCapability(descriptorNum, simultaneous,extCapability);            simultaneous = num;         }     simultaneous = P_MAX_INDEX;     basecapabilities.SetCapability(descriptorNum, simultaneous,new H323ControlExtendedVideoCapability());  !
>     delete capability;   }  } 
> void H323CodecExtendedVideoCapability::AddCapability(const PString & cap) {     extCapabilities.Add(H323ExtendedVideoFactory::CreateInstance(cap)); } 
> ------------------------------------------------- 
> I propose to replace the "void H323CodecExtendedVideoCapability::AddCapability(const PString & cap)" to this one: 
> void H323CodecExtendedVideoCapability::AddCapability(const PString & cap) {     extCapabilities.Add(H323ExtendedVideoFactory::CreateInstance(cap)->clone()); } 
> Thanks -- Iurii Gordiienko 

More information about the h323plus mailing list