2011-01-20 1 views
1

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

+1

Что такое 'blitBitmap()'? – chrisaycock

+3

Просто из любопытства: если вы кодируете это на C++, почему бы не использовать доступные вам функции, такие как new/delete и т. Д.? –

+3

..... это C++? Я вижу, у вас есть hodge podge c-style C++ и C++ - подумайте о том, чтобы инвестировать в хорошую книгу на C++! ;) 'malloc' является * faux-pas * в C++ land, рассмотрите использование' std :: vector' – Nim

ответ

6

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

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

должен быть

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

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

+0

Ничего себе. Дух, да, ха-ха. Интересно, как я это пропустил. ОК. Ну, теперь первые 3 строки - мусор, а все остальное в порядке. Должно быть, я все еще топаю в памяти. – jbrennan

+0

@jbrennan, после этого исправить это выглядит правильно для меня, отлично работает и для меня. Повторите попытку перекомпиляции из страйка и тестирования еще раз. –

+1

@jbrennan, я беру это обратно. См. Ответ Дэвида. Вы забыли пространство для нулевого терминатора. –

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

должен быть

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

и только тогда, когда вы кодирования C.

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

Vector< Vector <char> > 

если не делаете.

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

+0

Это для школы, и профессор не позволит нам использовать Ve ctors или других встроенных C++-данных. Это облом. – jbrennan

+0

@jbrennan: тогда это не C++, это C ... –

+0

@Matthieu No. Я бы обозначил его как C, за исключением того, что, знаете, я использую объекты. Более конкретно, конструкторы объектов. Мои вопросы частично основаны на не понимании того, как жизненный цикл объекта работает на C++. – jbrennan

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

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

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

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

0

здесь я думаю, что вы хотите SizeOf (Char *) внутри таНос

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

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

0

Ваш malloc() звонок не Посмотрите мне прямо, но, возможно, я чего-то не хватает.

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

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

+0

@jbrennan - Ах, тогда, возможно, единственная проблема была на самом деле размером, который вы выделяли. Обычно 'sizeof (char)' равно 1, а 'sizeof (char *)' равно 4 или 8. Таким образом, вы выделяете свой массив указателей строк слишком малым. –

1

В дополнение ко всем другим ответам:

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

+0

Спасибо за информацию. К сожалению, мне не разрешено использовать Vectors или C++ строки для этого задания (это для школы). – jbrennan

4

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

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

 Смежные вопросы

  • Нет связанных вопросов^_^