Memory loss with = operator

Question

Okay so I'm trying to implement a calculator on C++ that can process numbers of any size, for which I made a class named "bignum", so I'm not restrained by C++ default variables. The >> operator is supposed to read my input, parse what's read into an infix notation and then solve it. To parse the string I'm using Shunting Yard's method.

I was able able to solve the Segmentation Fault issues and it runs perfectly but I noticed I'm having memory problems with Valgrind:

==765== HEAP SUMMARY:
==765==     in use at exit: 2 bytes in 1 blocks
==765==   total heap usage: 10 allocs, 9 frees, 74,880 bytes allocated
==765==
==765== 2 bytes in 1 blocks are definitely lost in loss record 1 of 1
==765==    at 0x483C583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==765==    by 0x10E5A8: bignum::operator=(bignum const&) (bignum.cc:544)
==765==    by 0x110590: operator>>(std::istream&, bignum&) (bignum.cc:916)
==765==    by 0x10BB50: main (main.cc:110)
==765==
==765== LEAK SUMMARY:
==765==    definitely lost: 2 bytes in 1 blocks
==765==    indirectly lost: 0 bytes in 0 blocks
==765==      possibly lost: 0 bytes in 0 blocks
==765==    still reachable: 0 bytes in 0 blocks
==765==         suppressed: 0 bytes in 0 blocks
==765==
==765== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

The >> operator implementation that calls = operator:

// Input operator
istream& operator >> (istream &is, bignum &result)
{
    // Using de Shunting Yard's method
    stack<char> operations;
    queue<string> output;
    string input;
    
    // Pass the input to a string
    getline(is, input);

    // In case my string is empty
    if(input.empty())
        return is;

    // Parse the string
    for(size_t i = 0; i < input.size(); i++)
    {   
        if(isblank(input[i])){}

        // If a number is identified
        else if(isdigit(input[i]))
        {   
            size_t pos = i++;
            size_t len = 1;
            while(isdigit(input[i]))
            {
                len++;
                i++;
            }
            i--;
            output.push(input.substr(pos, len));
        }

        // If there's a minus sign
        else if(input[i] == '-')
        {
            size_t j = i;
            while(j != 0 && isblank(input[--j])){}

            // I've got a subtraction
            if(isdigit(input[j]) || input[j] == ')')
            {
                if(!operations.empty())
                {
                    while(operations.top() == '-' || operations.top() == '+' || operations.top() == '*' || operations.top() == '/')
                        output.push(string{operations.pull()});
                }   
                
                operations.push(input[i]);
            }
            // I've got a negative sign
            else
                output.push(string{"s"});
        }

        else if(input[i] == '+')
        {
            size_t j = i;
            while(j != 0 && isblank(input[--j])){}

            // I've got an addition
            if(isdigit(input[j]) || input[j] == ')')
            {
                if(!operations.empty())
                {
                    while(operations.top() == '-' || operations.top() == '+' || operations.top() == '*' || operations.top() == '/')
                        output.push(string{operations.pull()});
                }

                operations.push(input[i]);
            }
            // If there's a positive sign I do nothing
        }

        else if(input[i] == '*' || input[i] == '/')
        {   
            if(!operations.empty())
            {
                while(operations.top() == '*' || operations.top() == '/')
                    output.push(string{operations.pull()});
            }

            operations.push(input[i]);
        }

        // If there's an opening parenthesis
        else if(input[i] == '(')
            operations.push(input[i]);

        // If there's a closing parenthesis
        else if(input[i] == ')')
        {
            if(operations.empty())
            {   
                cout << "Syntax Error" << endl;
                return is;
            }
            
            while(!operations.empty() && operations.top() != '(')
                output.push(string{operations.pull()});

            if(!operations.empty())
                operations.pull();
            else
            {   
                cout << "Syntax Error" << endl;
                return is;
            }
        }
    }

    if(!operations.empty())
    {
        if(operations.top() == '(' && output.empty())
        {
            cout << "Syntax Error" << endl;
            return is;
        }
        else
        {   
            if(output.empty())
            {
                cout << "Syntax Error" << endl;
                return is;
            }
            else
            {   
                while(!operations.empty())
                    output.push(string{operations.pull()});
            }
        }
    }

    // I solve the output
    string aux;
    short_t sign = 0;
    stack<bignum> numbers;

    while(!output.empty())
    {
        aux = output.pull();

        if(isdigit(aux[0]))
        {
            numbers.push(bignum(aux, sign));
            sign = 0;
        }
        else if(aux[0] == 's')
            sign++;
        else if(aux[0] == '+')
        {   
            if(numbers.length() < 2)
            {   
                cout << "Syntax Error" << endl;
                return is;
            }
            result = numbers.pull();
            result = numbers.pull() + result;
            numbers.push(result);
        }
        else if(aux[0] == '-')
        {   
            if(numbers.length() < 2)
            {   
                cout << "Syntax Error" << endl;
                return is;
            }
            result = numbers.pull();
            result = numbers.pull() - result;
            numbers.push(result);
        }
        else if(aux[0] == '*')
        {   
            if(numbers.length() < 2)
            {   
                cout << "Syntax Error" << endl;
                return is;
            }
            result = numbers.pull();
            result = numbers.pull() * result;
            numbers.push(result);
        }
        else if(aux[0] == '/')
        {   
            if(numbers.length() < 2)
            {   
                cout << "Syntax Error" << endl;
                return is;
            }
            result = numbers.pull();
            result = numbers.pull() / result;
            numbers.push(result);
        }
    }

    if(!numbers.empty())
        result = numbers.pull();
    else
    {
        cout << "Syntax Error" << endl;
        return is;
    }

    return is;
}

My = operator:

// Constructor por copia
bignum& bignum::operator = (const bignum &right)
{   
    // Verifico si los bignum a igualar son distintos
    if(&right != this)
    {   
        // Borro el puntero de this
        delete[] digits;

        // En caso de que right este apuntando a NULL
        if(!right.digits)
        {   
            type = STANDARD;
            digits = NULL;
            size = 0;
            sign = 0;
        }

        else
        {   
            type = right.type;
            digits = new short_t[right.size];
            size = right.size;
            sign = right.sign;

            // Copio los valores del arreglo right en this
            for(size_t i = 0; i < size; i++)
                digits[i] = right.digits[i];
        }
    }

    // Devuelvo un puntero a mi bignum this
    return *this;
}

My class bignum:

class bignum
{
private:
    multiplication_algorithm_t type;
    short_t *digits;
    short_t sign;
    size_t size;

Please post a [mcve]. If it breaks with larger input, it can break with smaller input, unless the error is due to memory limitations or a buffer overrun of a fixed size array. — PaulMcKenzie, Oct 29 '21 at 15:22
I'm not using any fixed arrays and I'm guessing its not memory limitation either as larger inputs are solved correctly. The thing is that I cannot detect a pattern or similarity on the inputs that are not working to come with a minimal example. I'll edit the question to post some examples of inputs. — Sergio Lee, Oct 29 '21 at 15:29
What is `bignum`? A [mcve] means that we can take the code as-is, make no changes to it, compile it, and run it. — PaulMcKenzie, Oct 29 '21 at 15:42
Oh my bad, I'll try to get it done and get back to you as soon as I can, thanks! — Sergio Lee, Oct 29 '21 at 16:38
I think if you made the minimal but full example, then we can see where the crash occurs and possibly why. How to fix it would be a different story. — PaulMcKenzie, Oct 29 '21 at 16:42

Frebreeze · Answer 1 · 2021-10-29T15:24:35.537

0

So you are implementing a RPN calculator using a specific algorithm it sounds like.

Here is a SO RPN that is similar to your problem

General coding practices.

Try breaking up the code into smaller logical chunks.
Avoid long if, else-if statements. Consider using something like a switch.
You could consider using regex to identify the number and expression.
- See compilers lexers and parses for more info (more complicated)
Use a legitimate debugger.
- No, printf does not count

Guessing about your errors:

Improper array access. Look at places where you use ++, or -- counters. Likely a case is either accessing a negative index or exceeding past its array length.

For example:

else if(input[i] == '-')
        {
            size_t j = i;
            while(j != 0 && isblank(input[--j])){}

What if the first character of the input is a minus sign? '- 1 3'

edited Oct 29 '21 at 15:24

answered Oct 29 '21 at 15:05

Frebreeze

202
3
10

There's no out-of-bounds access in the code you've shown due to the `j != 0` check. – interjay Oct 29 '21 at 15:20
You are definitely right! My bad. – Frebreeze Oct 29 '21 at 15:23
I've been using gdb but the inputs are around a 1000 characters long so I'm not sure how to keep track to it, but I'll definitely check in my ++ or -- counters. – Sergio Lee Oct 29 '21 at 15:30
Sounds good. You can try creating your own input strings and test that way to find a broken case. Also, if you want to, look into CMake and GTest/Catch/Boost. Then you can write your own automated tests to verify work that you do. – Frebreeze Oct 29 '21 at 15:33

score 0 · Answer 2 · answered Oct 30 '21 at 22:04

0

I noticed I'm having memory problems with Valgrind

In your = operator you delete[] digits and allocate digits = new short_t[right.size] - you free the digits only on the next assignment and the last assigned digits stay allocated. You could free them in the destructor.

answered Oct 30 '21 at 22:04

Armali

18,255
14
57
171

1

I somehow managed to find the error with this!! Thank you soooooooooo much!! – Sergio Lee Oct 31 '21 at 18:21

score 0 · Answer 3 · answered Oct 31 '21 at 18:23

0

In case it helps I was aiming the pointer somewhere else before the destructor executed as my object was inside a loop!

answered Oct 31 '21 at 18:23

Sergio Lee

1
1

As it’s currently written, your answer is unclear. Please [edit] to add additional details that will help others understand how this addresses the question asked. You can find more information on how to write good answers [in the help center](/help/how-to-answer). – Community Oct 31 '21 at 22:12

Memory loss with = operator

3 Answers3