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

List:       helix-common-dev
Subject:    [Common-dev] CN-Client: prevent CHXBuffer from deleting
From:       "Henry Ping" <ping () real ! com>
Date:       2007-11-21 20:01:40
Message-ID: 008501c82c79$54a69f50$c49217ac () corp ! real ! com
[Download RAW message or body]

Fix has been checked in to Cay150, Cay203, Cay204, Atlas310, Atlas320 and
HEAD.

Thanks
Henry 

> -----Original Message-----
> From: Greg Wright [mailto:gwright@real.com]
> Sent: Wednesday, November 21, 2007 11:27 AM
> To: ping@real.com
> Cc: common-dev@helixcommunity.org
> Subject: Re: [Common-dev] RESEND CR-Client: prevent CHXBuffer from
> deleting data *not*owned by itself
> 
> 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]