[h323plus] Bug inH323Capabilities::Remove(H323Capability*)

Jan Willamowius jan at willamowius.de
Sun Jan 31 17:41:21 EST 2016


Hi Iurii,

good find! The thing with RemoveAt() is that it moves the remaining
content in the array down one position. So if we use it in a for
loop that will increment the index with each round, we'll skip the
item right after the deleted one if the loop counter isn't decremeted
after the delete.

I've put a fix into the CVS. Can you please test how it works for you ?

Regards,
Jan

-- 
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 found a bug - H323Capabilities::Remove function will skip item when doing .RemoveAt operation. In result I have had a crash (in line if (set[outer][middle][inner].GetCapabilityNumber() == capabilityNumber)) for some sets of capabilities. I can't understand why the crash is occurs but in any way... 
> 
> Original code: 
> void H323Capabilities::Remove(H323Capability * capability) {   if (capability == NULL)     return; 
>   PTRACE(3, "H323\tRemoving capability: " << *capability); 
>   unsigned capabilityNumber = capability->GetCapabilityNumber(); 
>   for (PINDEX outer = 0; outer < set.GetSize(); outer++) {     for (PINDEX middle = 0; middle < set[outer].GetSize(); middle++) {       for (PINDEX inner = 0; inner < set[outer][middle].GetSize(); inner++) {         if (set[outer][middle][inner].GetCapabilityNumber() == capabilityNumber) {           set[outer][middle].RemoveAt(inner);           break;         }       }       if (set[outer][middle].GetSize() == 0)         set[outer].RemoveAt(middle); 
> 
>     }     if (set[outer].GetSize() == 0)         set.RemoveAt(outer); 
>   } 
> 
> My fix: 
> void H323Capabilities::Remove(H323Capability * capability) {   if (capability == NULL)     return; 
>   PTRACE(3, "H323\tRemoving capability: " << *capability); 
>   unsigned capabilityNumber = capability->GetCapabilityNumber(); 
>   for (PINDEX outer = 0; outer < set.GetSize(); ) {     for (PINDEX middle = 0; middle < set[outer].GetSize(); ) {       for (PINDEX inner = 0; inner < set[outer][middle].GetSize(); inner++) {         if (set[outer][middle][inner].GetCapabilityNumber() == capabilityNumber) {           set[outer][middle].RemoveAt(inner);           break;         }       }       if (set[outer][middle].GetSize())         ++middle;       else         set[outer].RemoveAt(middle); 
>     }     if (set[outer].GetSize())         ++outer;     else         set.RemoveAt(outer);   } 
> Thanks -- Iurii Gordiienko 





More information about the h323plus mailing list