[Boost-bugs] [Boost C++ Libraries] #2200: Bad memory management in algorithm::is_any_of

Subject: [Boost-bugs] [Boost C++ Libraries] #2200: Bad memory management in algorithm::is_any_of
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2008-08-15 16:12:58

#2200: Bad memory management in algorithm::is_any_of
--------------------------------------+-------------------------------------
 Reporter:  jgottman_at_[hidden]  |        Type:  Bugs             
   Status:  new                       |   Milestone:  Boost 1.36.0     
Component:  None                      |     Version:  Boost 1.35.0     
 Severity:  Problem                   |    Keywords:  string algorithms
--------------------------------------+-------------------------------------
 The is_any_of predicate has been modified to use either a fixed buffer or
 dynamic memory allocation, depending on its size.  There are two major
 problems when it uses dynamic memory allocation.  See the code in
 boost_1_36_0\boost\algorithm\string\detail\classification.hpp.

    1) It allocates memory with the command
 {{{
         m_Storage.m_dynSet=new set_value_type[m_Size];
 }}}
       but it deallocates with the command
 {{{
         delete m_Storage.m_dynSet;
 }}}
      This should be
 {{{
         delete[] m_Storage.m_dynSet;
 }}}
      or else we will have undefined behavior.

    2) The first line of its assignment operator is
 {{{
       m_Storage.m_dynSet=0;
 }}}
      This means that if m_dynSet has had memory allocated that memory will
 leak. Also, this means that it is unsafe under self-assignment.

 Here is some untested code that should help with the assignment problem:

 {{{
 is_any_ofF& operator=(const is_any_ofF& Other)
 {
    const set_value_type* SrcStorage=0;
    set_value_type* DestStorage=0;
    if (Other.m_Size <= FIXED_STORAGE_SIZE) {
        SrcStorage = &Other.m_Storage.m_fixSet[0];
        if (m_Size > FIXED_STORAGE_SIZE) {
            m_Storage.m_dynSet  =  new set_value_typ[Other.m_Size];
            DestStorage = m_Storage.m_dynSet;
        } else {
            DestStorage = &m_Storage.m_fixSet[0];
        }
   } else {
        SrcStorage = Other.m_Storage.m_dynSet;
        if (m_Size<= FIXED_STORAGE_SIZE) {
           m_Storage.m_dynSet  =  new set_value_type[Other.m_Size];
        } else if (m_Size <= Other.m_Size) {
           //We already have enough space allocated, so no new allocation
 required
        } else {
           //Use temporary var for strong exception safety
           set_value_type *temp = new set_value_type[Other.m_Size];
           delete[] m_Storage.m_dynSet;
           m_Storage.m_dynSet = temp;
        }
        DestStorage = m_Storage.m_dynSet;
   }
   m_Size = Other.m_Size;
   ::memcpy(DestStorage, SrcStorage, sizeof(set_value_type)*m_Size);
   return *this;
 }

 }}}

-- 
Ticket URL: <http://svn.boost.org/trac/boost/ticket/2200>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.