[prev in list] [next in list] [prev in thread] [next in thread] 

List:       helix-common-dev
Subject:    Re: [Common-dev] RESEND CR-Client: prevent CHXBuffer from deleting
From:       Greg Wright <gwright () real ! com>
Date:       2007-11-21 19:27:15
Message-ID: 47448693.5070800 () real ! com
[Download RAW message or body]

Looks good.
--greg.


Henry Ping wrote:
> I will check in the fix at noon if there is no comment.
> 
> Henry
> 
>> -----Original Message-----
>> From: common-dev-bounces@helixcommunity.org [mailto:common-dev-
>> bounces@helixcommunity.org] On Behalf Of Henry Ping
>> Sent: Tuesday, November 20, 2007 12:32 PM
>> To: common-dev@helixcommunity.org
>> Subject: [Common-dev] CR-Client: prevent CHXBuffer from deleting data
>> *not*owned by itself
>>
>> Description
>> -----------------------------------
>> CHXBuffer has a special mode when the data is owned by another CHXBuffer,
>> this is indicated by m_bJustPointToExistingData.
>>
>> Inside CHXBuffer::SetSize(), there is logic to deallocate the original
>> buffer and allocate new buffer with the new size. But, it doesn't check
>> m_bJustPointToExistingData flag before deallocating the buffer, as a
>> result,
>> it can deallocate buffer which is not owned by itself and cause access
>> violation when the actual owner tries to access the same buffer.
>>
>> Such situation exists in our audio service where it leverages this special
>> mode in CHXBuffer to reduce memcpy and recycles the same CHXBuffer to
>> reduce
>> additional memory allocation during the playback.
>>
>> The fix involves special handling of this mode inside
>> CHXBuffer::SetSize(),
>> CHXBuffer will allocate its own buffer and copy existing data if
>> necessary.
>>
>> Files Modified
>> -----------------------------------
>> common/container/hxbuffer.cpp
>>
>> Branches
>> -----------------------------------
>> HEAD, 150Cayenne, 310Atlas and 320Atlas
>>
>> Note,
>> Though this fix is applicable for other branches, I will leave it to the
>> branch owner to decide whether to pick up this fix.
>>
>> Diffs:
>> Index: hxbuffer.cpp
>> ===================================================================
>> RCS file: /cvsroot/common/container/hxbuffer.cpp,v
>> retrieving revision 1.13.2.1
>> diff -u -w -4 -r1.13.2.1 hxbuffer.cpp
>> --- hxbuffer.cpp	28 Mar 2005 16:45:47 -0000	1.13.2.1
>> +++ hxbuffer.cpp	20 Nov 2007 20:02:37 -0000
>> @@ -327,8 +327,56 @@
>>          HX_ASSERT(!"CHXBuffer::SetSize() will bail out because refcount >
>> 1");
>>          return HXR_UNEXPECTED;
>>      }
>>
>> +    // special case when the current data is "owned" by another CHXBuffer
>> +    if (m_bJustPointToExistingData)
>> +    {
>> +        // we will own the data after SetSize() is called
>> +        m_bJustPointToExistingData = FALSE;
>> +
>> +        HX_ASSERT(!IsShort());
>> +        if (ulLength <= MaxPnbufShortDataLen)
>> +        {
>> +            if (copyExistingData)
>> +            {
>> +                memcpy(m_ShortData, m_BigData.m_pData, ulLength); /*
>> Flawfinder: ignore */
>> +            }
>> +            m_ShortData[MaxPnbufShortDataLen] = (UCHAR)ulLength;
>> +            m_BigData.m_pData = NULL;
>> +            m_ulAllocLength = 0;
>> +        }
>> +        else
>> +        {
>> +            UCHAR* pNewData = Allocate(ulLength);
>> +            if (!pNewData)
>> +            {
>> +                return HXR_OUTOFMEMORY;
>> +            }
>> +
>> +           m_ulAllocLength = ulLength;
>> +
>> +            if (copyExistingData)
>> +            {
>> +                if (m_BigData.m_ulLength <= m_ulAllocLength)
>> +                {
>> +                    memcpy(pNewData, m_BigData.m_pData,
>> m_BigData.m_ulLength); /* Flawfinder: ignore */
>> +                }
>> +                else
>> +                {
>> +                    memcpy(pNewData, m_BigData.m_pData, m_ulAllocLength);
>> /* Flawfinder: ignore */
>> +                }
>> +            }
>> +
>> +            m_BigData.m_pData = pNewData;
>> +            m_BigData.m_ulLength = ulLength;
>> +            m_BigData.m_FreeWithMallocInterfaceIfAvail = TRUE;
>> +        }
>> +    }
>> +    else
>> +    {
>> +        HX_ASSERT(!m_bJustPointToExistingData);
>> +
>>      if (ulLength <= GetSize())
>>      {
>>          if (IsShort())
>>          {
>> @@ -427,8 +475,9 @@
>>              // Set flag
>>              m_BigData.m_FreeWithMallocInterfaceIfAvail = TRUE;
>>          }
>>      }
>> +    }
>>
>>      return HXR_OK;
>>  }
>>
>>
>> Henry
>>
>>
>> _______________________________________________
>> Common-dev mailing list
>> Common-dev@helixcommunity.org
>> http://lists.helixcommunity.org/mailman/listinfo/common-dev
> 
> 
> _______________________________________________
> Common-dev mailing list
> Common-dev@helixcommunity.org
> http://lists.helixcommunity.org/mailman/listinfo/common-dev


_______________________________________________
Common-dev mailing list
Common-dev@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/common-dev
[prev in list] [next in list] [prev in thread] [next in thread]