Распределение памяти - PullRequest
Новая
       14

Распределение памяти

0 голосов
/ 16 ноября 2010

Для проекта мне нужно реализовать класс bitset. Мой код до сих пор:

Заголовочный файл 

#ifndef BITSET_H_
#define BITSET_H_
#include <string>
#include <cmath>

using namespace std;

// Container class to hold and manipulate bitsets
class Bitset {
public:
    Bitset();
    Bitset(const string);
    ~Bitset();

    // Returns the size of the bitset
    int size();

    // Sets a bitset equal to the specified value
    void operator= (const string);

    // Accesses a specific bit from the bitset
    bool operator[] (const int) const;

private:
    unsigned char *bitset;
    int set_size;
    // Sets a bitset equal to the specified value
    void assign(const string);
};
#endif /* BITSET_H_ */

Исходный файл 

#include "bitset.h"

Bitset::Bitset() {
    bitset = NULL;
}

Bitset::Bitset(const string value) {
    bitset = NULL;
    assign(value);
}

Bitset::~Bitset() {
    if (bitset != NULL) {
        delete[] bitset;
    }
}

int Bitset::size() {
    return set_size;
}

void Bitset::operator= (const string value) {
    assign(value);
}

bool Bitset::operator[] (const int index) const {
    int offset;

    if (index >= set_size) {
        return false;
    }

    offset = (int) index/sizeof(unsigned char);
    return (bitset[offset] >> (index - offset*sizeof(unsigned char))) & 1;
}

void Bitset::assign(const string value) {
    int i, offset;

    if (bitset != NULL) {
        delete[] bitset;
    }

    bitset = new unsigned char[(int) ceil(value.length()/sizeof(unsigned char))];

    for (i = 0; i < value.length(); i++) {
        offset = (int) i/sizeof(unsigned char);
        if (value[i] == '1') {
            bitset[offset] |= (1 << (i - offset*sizeof(unsigned char)));
        } else {
            bitset[offset] &= ~(1 << (i - offset*sizeof(unsigned char)));
        }
    }

    set_size = value.length();
}

Моя проблема - мои операторы удаления в деконструкторе и дампе ядра метода назначения. Разве нет необходимости освобождать эту память? Из того, что я прочитал до сих пор, всегда необходимо использовать команду удаления при каждом вызове new.

РЕДАКТИРОВАТЬ: Я изменил код выше, чтобы отразить одно из исправлений. Я добавил bitset = NULL в конструктор. Это исправило дамп ядра в методе assign, однако я по-прежнему получаю ошибки в деконструкторе.

Ответы [ 3 ]

4 голосов
/ 16 ноября 2010

Я думаю, вы должны инициализировать bitset до NULL во втором конструкторе.

Почему?

Поскольку переменная-указатель не обязательно будет инициализирована как NULL. Таким образом, вы можете пытаться delete[] какой-то случайный адрес памяти при использовании этого второго конструктора.

Итак, вы должны иметь: 

Bitset::Bitset(const string value) : bitset(NULL)
{
    assign(value);
}
1 голос
/ 16 ноября 2010

Скорее всего, вы копируете Bitset куда-нибудь. Вы не определили конструктор копирования, а не оператор назначения копирования. Результатом копирования является то, что у вас есть два экземпляра, которые оба считают, что они должны освободить динамически распределенный массив, когда они закончат.

Это известно как Правило трех : если вы определяете какой-либо из деструктора, конструктора копирования или оператора копирования, скорее всего, вам нужно будет определить все три .

Теперь о вашем коде: 

#include "bitset.h"

OK. 

Bitset::Bitset() {
    bitset = NULL;
}

(1) Вы не включили заголовок, который гарантированно определяет NULL.

(2) вы не инициализируете элемент set_size, поэтому проверка в операторе индекса может / будет использовать неопределенное значение с неопределенным поведением.

(3) обычно предпочитают использовать список инициализаторов, а не присваивание (это позволяет избежать, например, построения по умолчанию с последующим присваиванием). 

Bitset::Bitset(const string value) {
    bitset = NULL;
    assign(value);
}

(4) Обычно не очень хорошая идея выражать конструкцию в терминах назначения. Вместо этого выражайте назначение с точки зрения строительства. 

Bitset::~Bitset() {
    if (bitset != NULL) {
        delete[] bitset;
    }
}

(5) Проверка для NULL не требуется; Вы можете безопасно delete нулевой указатель. 

int Bitset::size() {
    return set_size;
}

(6) Ну, ну, set_size был участником, который не был инициализирован & hellip; Кроме того, эта функция-член должна быть const. 

void Bitset::operator= (const string value) {
    assign(value);
}

(7) Оператор присваивания должен вообще возвращать ссылку на присвоенный объект. Это просто соглашение, но это то, чего ожидают пользователи вашего класса.

(8) Передайте входной аргумент по значению или по ссылке на const. Обычно для встроенных типов выбирайте по значению, а для других типов, таких как std::string, выберите ссылку на const. То есть формальный аргумент должен быть лучше string const& value. 

bool Bitset::operator[] (const int index) const {
    int offset;

    if (index >= set_size) {
        return false;
    }

    offset = (int) index/sizeof(unsigned char);
    return (bitset[offset] >> (index - offset*sizeof(unsigned char))) & 1;
}

(9) Сначала опять неинициализированный set_size член.

(10) Затем обратите внимание, что sizeof(unsigned char) равно 1 по определению. Вы, вероятно, хотите использовать CHAR_BIT из <limits.h> здесь. Или просто используйте 8, если вы не планируете поддержку компьютеров Unisys (9-битный байт) или, возможно, цифровой сигнальный процессор Texas Instruments (16-битный байт). 

void Bitset::assign(const string value) {
    int i, offset;

    if (bitset != NULL) {
        delete[] bitset;
    }

(11) Проверка для NULL не требуется. 

    bitset = new unsigned char[(int) ceil(value.length()/sizeof(unsigned char))];

(12) Как уже упоминалось, sizeof(char) равно 1 по определению.

(13) Деление имеет целочисленные аргументы и поэтому является целочисленным делением, а не делением с плавающей запятой. Предположительно, что вы хотите, это трюк (a+b-1)/b? 

    for (i = 0; i < value.length(); i++) {

(14) Стиль: объявите переменную как можно ближе к ее первому использованию, насколько это практически возможно. Здесь это означает объявление счетчика цикла i непосредственно в заголовке цикла, например: for( int i = 0, .... 

        offset = (int) i/sizeof(unsigned char);

(14) И то же самое для offset. Но для этой переменной вы не планируете изменять ее значение, поэтому также объявите ее const. 

        if (value[i] == '1') {
            bitset[offset] |= (1 << (i - offset*sizeof(unsigned char)));
        } else {
            bitset[offset] &= ~(1 << (i - offset*sizeof(unsigned char)));
        }

(15) Лучше переосмыслить эти операции смены! 

    }

    set_size = value.length();
}

Приветствия & hth.,

0 голосов
/ 16 ноября 2010

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

...