Правильно ли я удаляю свою структуру? - PullRequest
0 голосов
/ 03 ноября 2010

Это домашнее задание. Контейнер Field был назначением недели назад, и теперь я должен использовать контейнер Field, чтобы действовать в качестве динамического массива для структуры NumPair, которая содержит два char * примерно так:

struct NumPair
{
 char *pFirst, *pSecond;
 int count;

 NumPair( char *pfirst = "", char *psecond = "", int count = 0)
  : pFirst(strdup(pfirst)), pSecond(strdup(psecond)), count(count)
 { }

NumPair( const NumPair& np )
    : count(np.count), pFirst(strdup(np.pFirst)), pSecond(strdup(np.pSecond))
{   }

NumPair& operator=( const NumPair& np )
{
    if(this != &np)
    {
        pFirst  = strdup(np.pFirst);
        pSecond = strdup(np.pSecond);
        count = np.count;
    }

    return *this;
}

и полевой контейнер

Field<NumPair> dict_;

Домашняя работа требует использования char *, а не строки, чтобы мы могли поправиться со всем этим материалом низкого уровня. У меня уже был вопрос по поводу преобразования char в wchar_t и т. Д.

Теперь у меня есть вопрос, правильно ли я уничтожаю NumPair. Сценарий таков:

1) Деструктор поля вызывается

template <class T>
Field<T>::~Field()
{
 delete[] v_;
}

2) Delete вызывает деструктор каждого элемента NumPair в v _;

~NumPair()
{
 free(pFirst);
 free(pSecond);
}

Это хорошо? На самом деле я не читал слишком много статей о смешивании и сопоставлении элементов, созданных в куче и бесплатном хранилище, как мы хотим. Я полагаю, что пока я не использую delete для неправильного элемента malloc, все будет в порядке.

Однако я не знаю всех тонкостей команды удаления, поэтому мне интересно, является ли это допустимым проектом, и что я мог бы сделать, чтобы сделать его лучше.

Кроме того, конечно, это не так. Я получаю сообщение об ошибке типа:

This may be due to a corruption of the heap и указывает на dbgheap

extern "C" _CRTIMP int __cdecl _CrtIsValidHeapPointer(
  const void * pUserData
  )
{
  if (!pUserData)
   return FALSE;

  if (!_CrtIsValidPointer(pHdr(pUserData), sizeof(_CrtMemBlockHeader), FALSE))
   return FALSE;

  return HeapValidate( _crtheap, 0, pHdr(pUserData) ); // Here
}

Опять же, как я могу улучшить это без использования строки?

ПОЛЕ CTOR / Копировать Ctor / Назначение

template <class T>
Field<T>::Field()
    : v_(0), vused_(0), vsize_(0)
{ }

template <class T>
Field<T>::Field(size_t n, const T &val)
    : v_(0), vused_(n), vsize_(0)
{
    if(n > 0)
    {
        vsize_ = 1;
        while(vsize_ < n)
            vsize_ <<= 1;

        v_     = new T[vsize_];
        std::fill(v_, (v_ + vused_), val);
    }
}

template <class T>
Field<T>::Field(const Field<T> &other)
    : v_(new T[other.vsize_]), vsize_(other.vsize_), vused_(other.vused_)
{ 
    std::copy(other.v_, (other.v_ + other.vused_), v_);
}

template <class T>
Field<T>& Field<T>::operator =(const Field<T> &other)
{
    this->v_     = other.v_;
    this->vused_ = other.vused_;
    this->vsize_ = other.vsize_;

    return *this;
}

ЧЛЕНЫ ПОЛЯ

T *v_;
size_t vsize_;
size_t vused_;

Ответы [ 3 ]

1 голос
/ 04 ноября 2010

Ваш конструктор копирования для Field просто копирует указатели в v_.Если у вас есть две копии поля, все NumPairs в v_ будут удалены, когда первое поле выйдет из области видимости, а затем снова будут удалены, когда появится второе.

1 голос
/ 04 ноября 2010

Ваш конструктор копирования (из поля <>) выглядит нормально, но operator= проблематичен.

Это не только утечка памяти (что происходит с исходным v_?), Но и после этого два экземпляра Field <> содержат указатель на один и тот же блок памяти, а тот, который уничтожен первым, сделает недействительным остальные v_ - и вы даже не можете сказать, произошло ли это.

Не всегда легко решить, что делать с operator= - некоторые думают, что неявная семантика перемещения в порядке, но остальные из нас видят, как это с большинством людей, с std::auto_ptr. Вероятно, самое простое решение - полностью отключить копирование и использовать явные функции для перемещения владельца.

1 голос
/ 03 ноября 2010

Ваша обработка строк в NumPair выглядит нормально (strdup + free), а удаление контейнера Field [] выглядит нормально, но трудно сказать, потому что вы не показываете, что такое v_.

eq упоминает в комментарии, что вам также следует остерегаться того, как вы копируете NumPairs. По умолчанию C ++ предоставит вам неявный конструктор копирования по элементам. Вот где RAII-тип, такой как std :: string, делает вашу жизнь проще: ваш std :: string, содержащий struct, может быть скопирован без какой-либо специальной обработки с вашей стороны, а память, на которую ссылается строка, будет обрабатываться строкой копия. Если вы продублируете свой NumPair (например, назначив его или вернув из функции), уничтожение временного файла освободит ваши строки из-под вас.

...