Sysmalloc error on vector pointer on second push back

Question

For a school assignment I need to keep track of multiple instances of a Product class.

Within the main of my program I am making a vector as follows:

std::vector<std::unique_ptr<Product>> products;

On a specific condition within my main, I am passing this vector as a pointer to another function called processData. This one is looking like this (simplified as much as possible):

void processData(std::vector<std::unique_ptr<Product>>* products)
{
while (true)
{
    //Do something with std::cin
    //Do something with that data
    products->push_back(std::unique_ptr<Product>(new Product(dataX, dataB)));
}
}

What happens

The first push back goes well. It is doing what it's supposed to do. However, calling the push_back for the second time causes the following error:

main: malloc.c:2385: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.

My thoughts about this

I think that the pointer to the vector is being changed after executing the first push_back command. Because the device has to reallocate memory. So while wanting to execute the second push_back, the pointer is pointing to an invalid location.

What I tried to do to solve this

I tried using emplace_back function, but I already knew that would not work.

Within the main of my program I am currently executing the following code, before calling the processData function:

products.reserve(100);

Why?

Because I thought this would allocate enough space for 100 Product pointers, so it would not reallocate memory.

Currently I am still having the issue as described above. I hope you guys can point me in the right direction.

Before using the code

You will need to input some data. The data is looking like this:

#Data|<six numbers>|<two numbers>%

Example:

#Data|123456|12%

After putting in multiple ones, it will crash. It seems like removePrefix is the causer, not sure.

#include <iostream>
#include <cstdio>
#include <vector>
#include <algorithm>
#include <memory>

#define END_MSG                 "End"
#define PRODUCT_NUMBER_LENGTH   6
#define AMOUNT_OF_SEPERATORS    2
#define STRING_BEGIN_INDEX      0

struct Product{
    int productNumber;
    int quantity;
    Product(int productNumber, int quantity) : productNumber(productNumber), quantity(quantity)
    {   
    }
};


void removePrefix(std::string* msg)
{
    if (msg != nullptr)
    {
        msg->erase(msg->begin());
        msg->erase(msg->end());
    }
}

void processData(std::vector<std::unique_ptr<Product>>& products)
{
    std::string DATA_MSG = "Data";
    std::string data;

    bool process = true;
    while (process)
    {
        std::cin >> data;
        removePrefix(&data);
        if (data.compare(END_MSG) != 0)
        {
            if (data.find(DATA_MSG) != std::string::npos &&
                    std::count(data.begin(), data.end(), '|') == AMOUNT_OF_SEPERATORS)
            {
                data.erase(STRING_BEGIN_INDEX, DATA_MSG.length() + 1); //including the first "|"
                int productNumber = std::stoi(data.substr(STRING_BEGIN_INDEX, PRODUCT_NUMBER_LENGTH));
                int quantity = std::stoi(data.substr(PRODUCT_NUMBER_LENGTH + 1));

                products.push_back(std::unique_ptr<Product>(new Product(productNumber, quantity)));
            }
        }
        else
        {
            process = false;
        }
    }
}


int main()
{
    std::string input;
    std::vector<std::unique_ptr<Product>> products;
    products.reserve(100);

    while (true)
    {
        processData(products);
    }
    return 0;
}

Answer 1

It seems like removePrefix is the causer, not sure.

void removePrefix(std::string* msg)
{
    if (msg != nullptr)
    {
        msg->erase(msg->begin());
        msg->erase(msg->end());
    }
}

msg->end() is an iterator one past the last character in the string. It does not refer to a character that you could remove from the string. Further you do check for nullptr but you do not check for the size of the string. If the string is empty, then begin() == end() and both calls will attempt to erase something which isn't there.

Please do not pass pointers everywhere when passing a nullptr does not make sense. If you do not have a string then you would not call the function, so you never need to call it with a nullptr. Use references instead:

 void removePrefix(std::string& msg)
 {
     if (msg.size() >= 2) {
         msg.erase(msg.begin());
         msg.erase(msg.end() - 1);
     }
 }

PS: You have another call to erase:

data.erase(STRING_BEGIN_INDEX, DATA_MSG.length() + 1); 

Also here I don't see that you make sure that the indices are in range. However, this will throw a std::out_of_range exception, which isnt the case for the iterator overload (see here).

Answer 2

I think your problem is that unique_ptr has no copy constructor: Why can I not push_back a unique_ptr into a vector?

push_back works as long as the instance can be moved. For emplace_back the instance is created directly. https://godbolt.org/z/26CHVj

-- edit: as idclev 463035818 noted, this would of course lead to a complile error and not a runtime error. And yes, push_back has an r-reference overload and my statement about this is accordingly fuzzy and I have adjusted it.