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

Question

Я новичок в C ++, у меня большой опыт работы с Objective-C.

Я пытаюсь получить массив 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 ++. В противном случае, да, я знаю, что я должен использовать их. Спасибо за все предложения.

Answer 1

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

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

должен быть

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

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

Answer 2

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

должно быть

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

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

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

Vector< Vector < char > > 

, если вы этого не сделаете.

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

Answer 3

Этот 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, просто назначьте символ напрямую:

    for (int currentColumn = 0; currentColumn < width; currentColumn++) {
        _bitmap[currentRow][currentColumn] = 'a';
    }
    _bitmap[currentRow][width] = 0; // and don't forget to terminate the string

Answer 4

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

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);

Answer 5

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

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> в этом случае отлично послужит цели.

Answer 6

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

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

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

Answer 7

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

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

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

Answer 8

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

_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]); 
} 

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