Странные проблемы управления памятью в C++ (по крайней мере, от новичка)

Я новичок в C++, у меня много опыта в Objective-C.

Я пытаюсь получить массив с-строк (то есть char **) в качестве переменной экземпляра в моем классе, которая выделяется и заполняется в моем конструкторе, а затем в другой функции-члене я хочу распечатать всю "сетку".

Распределение работает, я заполняю свой массив строками (пока просто "aaaaaaa" и так далее). Проверяя в конце моего конструктора, я вижу, что каждая строка была успешно создана и заполнена, как и ожидалось.

Однако затем я вызываю свою функцию printGrid(), и затем все становится странным. Если у меня есть 25 строк для печати, скажем, первые 12 или около того будут печатать мусор, а остальные 13 распечатать, как ожидалось. Так что кажется, что я где-то попираю память, и я не уверен, где.

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

main.cpp: где я вызываю функции

#include <iostream>
#include "Bitmap.h"

using namespace std;
int main (int argc, char * const argv[]) {

    Bitmap bitmap(15, 25);
    bitmap.printBitmap();

    return 0;
}

Bitmap.h: заголовок для моего класса

class Bitmap {
private:
    char **_bitmap;
        void printLine(char const*lineString);
    int _width;
    int _height;
public:
    Bitmap();
        Bitmap(int width, int height);
    void printBitmap();
};

Bitmap.cpp: где происходит действие

#include <iostream>
#include "Bitmap.h"

using namespace std;
Bitmap::Bitmap() {
    // allocate space for the bitmap
    int numRows = 20;
    int numColumns = 30;

    Bitmap(numRows, numColumns); // Can I even safely do this? I'm not using the default constructor in my main() but I'm still curious.
}

Bitmap::Bitmap(int width, int height) {
    _width = width;
    _height = height;

    _bitmap = (char **)malloc(sizeof(char*) * height); // FIXED this line (used to be char, now it's char *).
    for (int currentRow = 0; currentRow < height; currentRow++) {
        _bitmap[currentRow] = (char *)malloc((sizeof(char) * width));
        snprintf(_bitmap[currentRow], width, "%s", "1");

        for (int currentColumn = 0; currentColumn < width; currentColumn++) {
            _bitmap[currentRow] = strcat(_bitmap[currentRow], "a");
        }
        printf("currentRow %0d: %s\n",currentRow, _bitmap[currentRow]); // Each row prints out FINE here, as expected
    }
}

void Bitmap::printBitmap() {
    int numColumns =_width;
    int numRows = _height;

    if (NULL == _bitmap)
        return;

    // iterate over the bitmap, line by line and print it out
    for (int currentRow = 0; currentRow < numRows; currentRow++) {

        // If there are, say, 25 lines, the first 12 or so will be garbage, then the remaining will print as expected
        printLine((char const *)_bitmap[currentRow]);
    }
}

void Bitmap::printLine(char const*lineString) {
    printf(":%s\n", lineString);    
}

Это для школы, и проф не допускает векторы или строки C++. В противном случае, да, я знаю, что я должен использовать их. Спасибо за все предложения.

Источник

8 ответов

Решение

Красный флаг:

_bitmap = (char **)malloc(sizeof(char) * height);

должно быть

_bitmap = (char **)malloc(sizeof(char*) * height);

Вы хотите указатель на char*, а не указатель на char,

Источник 
_bitmap = (char **)malloc(sizeof(char) * height);

должно быть

_bitmap = (char **)malloc(sizeof(char*) * height);

и только если вы кодируете C.

Лучше использовать new/delete, если вам абсолютно необходимо, чтобы растровое изображение было смежным, и

Vector< Vector < char > >

если нет

Кроме того, strcat кажется странным выбором, учитывая, что вы еще не инициализировали память. Т.е. здесь не обязательно 0, поэтому нет конца строки. Это может привести к потере памяти. Попробуйте использовать strcpy (или strncpy, если вы хотите быть в безопасности).

Источник

Связанный с этим комментарием в вашем конструкторе по умолчанию:

Bitmap(numRows, numColumns); // Can I even safely do this? I'm not using
                             // the default constructor in my main() but
                             // I'm still curious.

Это не делает то, что вы думаете, что делает. Это не вызов другого конструктора для дополнительной инициализации. Вместо этого это создает другой временный неназванный Bitmap использование объекта numRows а также numColumnsи тут же вызывает его деструктор. Этот оператор действует как локальная переменная без имени.

В вашем случае вы можете предоставить конструктор по умолчанию, указав аргументы по умолчанию для одного конструктора:

public:
    Bitmap(int width = 20, int height = 30);
Источник

Этот malloc не оставляет места для байта 0 в конце строки:

    _bitmap[currentRow] = (char *)malloc((sizeof(char) * width));

Поскольку sizeof(char) равен 1 по определению, вы можете просто сделать:

    _bitmap[currentRow] = (char *)malloc(width+1);

И в этой конструкции:

    for (int currentColumn = 0; currentColumn < width; currentColumn++) {
        _bitmap[currentRow] = strcat(_bitmap[currentRow], "a");
    }

ты не хочешь использовать strcat, просто назначьте char напрямую:

    for (int currentColumn = 0; currentColumn < width; currentColumn++) {
        _bitmap[currentRow][currentColumn] = 'a';
    }
    _bitmap[currentRow][width] = 0; // and don't forget to terminate the string
Источник

В дополнение ко всем остальным ответам:

Bitmap::Bitmap() {
    // allocate space for the bitmap
    int numRows = 20;
    int numColumns = 30;

    Bitmap(numRows, numColumns); // Can I even safely do this? I'm not using the default constructor in my main() but I'm still curious.
}

Нет, ты не можешь этого сделать. Каждый конструктор независим, и они не могут делегировать друг другу.

Для управления памятью используйте специальные классы управления ресурсами, которые будут автоматически управлять памятью. Стандарт предоставляет отличную серию классов и std::vector<std::string> будет служить цели в этом случае превосходно.

Источник

Здесь я думаю, что вы хотите sizeof (char *) внутри malloc

_bitmap = (char **)malloc(sizeof(char) * height);

Кроме того, когда вы заполняете строку символом "а", вы должны убедиться, что не перезаписываете какую-либо память: вы выделили символ ширины, вы напечатали ему "1", а затем конкатенировали "a" раз ширины, что будет равно 1 мимо выделенной памяти (не говоря уже о том, чтобы не оставить места для нуль-терминации

Источник

Ваш malloc() Звонок мне не подходит, но, возможно, я что-то упустил.

То, что я должен видеть, это один malloc() вызов для хранения для массива. Если вы хотите 10 строк C в нем, это было бы malloc(10 * sizeof (char *)), Тогда я должен увидеть еще malloc() вызовы, которые фактически выделяют память, которую используют сами 10 строк.

Но я вижу только один вызов malloc(), который думает, что он выделяет память массива строк, а не память массива указателей строк.

Источник

Следующее должно распределяться правильно (еще не проверял).

_bitmap = new char*[height];
for (int currentRow = 0; currentRow < height; currentRow++) 
{         
    _bitmap[currentRow] = new char[width];         
    snprintf(_bitmap[currentRow], width, "%s", "1");          
    for (int currentColumn = 0; currentColumn < width; currentColumn++) 
    {             
        _bitmap[currentRow] = strcat(_bitmap[currentRow], "a");         
    }                 // Each row prints out FINE here, as expected     

    printf("currentRow %0d: %s\n",currentRow, _bitmap[currentRow]); 
}

Также убедитесь, что вы определили конструктор копирования, деструктор и оператор присваивания, чтобы не допустить утечки памяти и удаления массива.

Источник
Другие вопросы по тегам