2013-02-28 3 views
7

У меня возникла проблема, когда мой код прервался при попытке использовать функцию размера() списка. По рекомендации stackoverflow :-) Я построил минимальный случай, в котором происходит segfault (в вызове inventory.size() ниже). Это:Настройка массива в конструкторе означает сбой позже

#include <list> 

class Thing {}; 

class Player { 
private: 
    int xpCalcArray[99]; 
    std::list<Thing*> inventory; 

public: 
    Player(); 

    int addToInv(Thing& t); // return 1 on success, 0 on failure 
}; 

Player::Player() { 
    // set up XP calculation array 
    for (int i=1; i<100; i++) { 
    if (i<=10) { 
     xpCalcArray[i] = i*100; 
    } 
    if (i>10 && i<=50) { 
     xpCalcArray[i] = i*1000; 
    } 
    if (i>50 && i<=99) { 
     xpCalcArray[i] = i*5000; 
    } 
    } 
} 

int Player::addToInv(Thing& t) { 
    if (inventory.size() == 52) { 
    return 0; 
    } else { 
     inventory.push_back(&t); 
    } 
    return 1; 
} 

int main(int argc, char *argv[]) { 
    Thing t; 
    Player pc; 
    pc.addToInv(t); 
    return 1; 
} 

Я заметил, что когда я извлекаю создание массива в cosntructor игрока, он отлично работает, так что это выглядит проблема. Что я делаю не так?

+2

+1: TestCase! Отлично! –

ответ

2

Доступ к внешней границе вашего массива. Это приводит к неопределенному поведению и поэтому нет логического объяснения чего-либо, что происходит после этого. Размер вашего массива равен 99, поэтому последний индекс равен 98. Однако ваш цикл for достигает 99.

Либо сделайте ваш размер массива 100:

int xpCalcArray[100]; 

Или изменить for условие i < 99.

4

Вы получаете доступ к вашему массиву за пределами границ, что приводит к неопределенным поведением. Допустимый диапазон индекса для этого массива

int xpCalcArray[99]; 

составляет от 0 до 98. Вы обращаетесь к индексу 99 здесь:

if (i>50 && i<=99) { 
    xpCalcArray[i] = i*5000; 
} 

Ваш внешний контур должен быть

for (int i=0; i<99; i++) { ... } 

Примечание I начинаются с 0 , хотя это предположение, что вы действительно хотите получить доступ к первому элементу.

Тогда ваше окончательное условие может быть упрощено до

if (i>50) { 
    xpCalcArray[i] = i*5000; 
} 

Если вы намеревались использовать массив размером 100, то вам нужно

int xpCalcArray[100]; 

затем петлю между int i=0; i<100;.

+2

И, очевидно, можно было бы использовать * единственную константу *, а не жестко-кодирующую 50, 98, 99 и 100 всюду ... –

+0

Спасибо всем. Doh. – KHekkus

2

Вы переписываете свой массив из 99 int s, пытаясь изменить 2-й → 100-й элемент (вместо 1-го → 99-го).

В вашем случае, это происходит перезапись некоторой памяти в пределах std::list<Thing*> (который существует в памяти непосредственно после массива — не всегда, но, видимо, для вас сегодня), и таким образом, когда вы пытаетесь использовать этот список, все ад распадается, когда его внутренние данные членов больше не то, что, как он думал.

1

Вы xpCalcArray определяется от 0 до 98 (из них 99 элементов).

Ваша петля проходит от 0 до 99, выполняя 100 шагов.

Последний цикл цикла, пишет xpCalcArray в местоположении 99, которого не существует. Это (косвенно) приводит к ошибке сегментации, как показано на рисунке the answer of Lightness Races in Orbit.

Таким образом, увеличение размера xpCalcArray на 1:

int xpCalcArray[100]; 
+0

Это _indirectly_ приводит к ошибке сегментации. Единственное прямое следствие здесь - использование UB. –

+0

Да, вы правы, ваше дополнительное объяснение причины превосходно! – Veger

+0

Не зная предполагаемой логики кода, увеличение размера массива не обязательно является правильным решением. Это может быть, но мы не можем быть уверены. – juanchopanza

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