Stack smashing detected

Question

#include <iostream>
using namespace std;
int main()
{
    int tablica[9];
    string inputromanum;
    cout << "ROMAN: ";
    cin >> inputromanum;
    int maxindeks;
    bool disablenextcomp = false;
    int readysolution = 0;
    maxindeks = inputromanum.length() - 1;{}{}
    for (int i = 0; i <= maxindeks; i++)
    {
        if (inputromanum[i] == 'M' || inputromanum[i] == 'm')
        {
            tablica[i] = 1000;
        }
        if (inputromanum[i] == 'D' || inputromanum[i] == 'd')
        {
            tablica[i] = 500;
        }
        if (inputromanum[i] == 'C'|| inputromanum[i] == 'c')
        {
            tablica[i] = 100;
        }
        if (inputromanum[i] == 'L' || inputromanum[i] == 'l')
        {
            tablica[i] = 50;
        }
        if (inputromanum[i] == 'X' || inputromanum[i] == 'x')
        {
            tablica[i] = 10;
        }
        if (inputromanum[i] == 'V' || inputromanum[i] == 'v')
        {
            tablica[i] = 5;
        }
        if (inputromanum[i] == 'I' || inputromanum[i] == 'i')
        {
            tablica[i] = 1;
        }
    }
    cout<<endl;
    for(int i4 = 0; i4 <= maxindeks; i4++)
    {
        cout<<"tablica["<<i4<<"] = "<<tablica[i4]<<endl;
    }
    for (int i2 = 0; i2 <= maxindeks; i2++)
    {
        int i5 = i2 + 1;
        if (i5 <= maxindeks)
        {
            //cout<<endl<<"tablica[i2 + 1] = "<<tablica[i2 + 1];
            //cout<<endl<<"tablica[i2] = "<<tablica[i2];
            //cout<<endl<<"tablica[i2 + 1] - tablica[i2] = "<<tablica[i2 + 1] - tablica[i2];
            if (tablica[i2 + 1] - tablica[i2] > 0 && disablenextcomp == false)
            { 
                //cout<<endl<<"readysolution + (tablica[i2 + 1] - tablica[i2]) = "<<readysolution + (tablica[i2 + 1] - tablica[i2])<<endl;
                readysolution = readysolution + (tablica[i2 + 1] - tablica[i2]);
                disablenextcomp = true;
            }
            else
            {
                if(disablenextcomp == false)
                {
                    //cout<<endl<<"readysolution + tablica[i2] = "<<readysolution + tablica[i2]<<endl;
                    readysolution = readysolution +  tablica[i2];
                }
                else
                {
                    disablenextcomp = false;
                }
            }
        }
        else
        {
            if(disablenextcomp == false)
            {
                //cout<<endl<<endl<<"OSTATNI INDEKS";
                //cout<<endl<<"tablica[i2] = "<<tablica[i2];
                //cout<<endl<<"readysolution + tablica[i2] = "<<readysolution + tablica[i2];
                readysolution = readysolution +  tablica[i2];
            }
        }
        i5++;
    }
    cout << endl << readysolution;
}

This is my program. made for decoding roman numerals into arabic ones. It works as intended in most cases, however, one of my colleagues found it to produce this error while inputting MMMCMXCVIII into the program:

*** stack smashing detected ***: terminated

It would refuse to work afterwards.

I wasn't able to find different numbers that would cause this error except MMMMMMMMMMM. It seems to fail when the index of tablica array exceeds 10. I don't know why it does so, as i am a novice in c++. It should've outputted 3999 instead of the error appearing. The numbers it should process successfully should range from 1 to 5000.

Answer 1

Thanks to folks in the comments, I've found the cause. The tablica[9] array is supposed to store 9 or less characters. The length of the input (MMMCMXCVIII in this case) has more characters, therefore it makes the for loop responsible for storing values for each character to cause mentioned above error, as there are no remaining units to store the values in. I've expanded the storage of tablica to 25 characters.

Answer 2

In modern C++ it is considered bad practice to use C-style arrays and index loops whenever you can avoid this. So, fo example you can rewrite first loop like this:

    std::vector<int> tablica;
    tablica.reserve(inputromanum.size()); // This line is not necessary, but it can help optimize memory allocations
    for (char c : inputromanum)
    {
        if (c == 'M' || c == 'm')
        {
            tablica.push_back(1000);
        }
        if (c == 'D' || c == 'd')
        {
            tablica.push_back(500);
        }
        if (c == 'C'|| c == 'c')
        {
            tablica.push_back(100);
        }
        if (c == 'L' || c == 'l')
        {
            tablica.push_back(50);
        }
        if (c == 'X' || c == 'x')
        {
            tablica.push_back(10);
        }
        if (c == 'V' || c == 'v')
        {
            tablica.push_back(5);
        }
        if (c == 'I' || c == 'i')
        {
            tablica.push_back(1);
        }
    }

And you will avoid your issue completly. Something similar can be done with other loops too. This approach also has benefit of (somewhat) properly handling situations when input line has other symbols, which is not roman number. Try it on your version and you will see what I mean.

One more point. When you need to do something different depending of value of one variable, like you did with all those ifs. There is special statement in C/C++ for this: switch. So instead of those ifs you can do this:

    std::vector<int> tablica;
    tablica.reserve(inputromanum.size()); // This line is not necessary, but it can help optimize memory allocations
    for (char c : inputromanum)
    {
        switch(c)
        {
            case 'M':
            case 'm':
                tablica.push_back(1000);
                break;
            case 'D':
            case 'd':
                tablica.push_back(500);
                break;
            case 'C':
            case 'c':
                tablica.push_back(100);
                break;
            case 'L':
            case 'l':
                tablica.push_back(50);
                break;
            case 'X':
            case 'x':
                tablica.push_back(10);
                break;
            case 'V':
            case 'v':
                tablica.push_back(5);
                break;
            case 'I':
            case 'i':
                tablica.push_back(1);
                break;
        }
    }