CODEWITHSUNDEEP
c++ifstreamunique-ptr
Matthew Bailey
Matthew Bailey

Reputation: 13

unique_ptr read access violation when losing scope

I'm reading a char array from a binary file using std::ifstream, then reinterpreting the char array into a pointer to my struct then making this a unique_ptr. Everything works well except when the unique_ptr goes out of scope I get a read access violation.

Am I going about this all wrong? I'm not sure why the error is occurring. I only have one unique_ptr on that data.

I've included the very basic version of the code to produce the error.

struct mystruct {
    int val = 0;
    double val2 = 5.6;
    std::string string = "test";

};

int main()
{

    //brackets here just to encapsulate the scope for this example
    {
        //try to open the stream for reading
        std::ifstream stream;
        stream.open("test.bin", std::ios::binary | std::ios::in);

        char* buffer = new char[sizeof(mystruct)];
        stream.read(buffer, sizeof(mystruct));

        mystruct * pointer = reinterpret_cast<mystruct*>(buffer);
        std::unique_ptr<mystruct> obj = std::unique_ptr<mystruct>(pointer);

        //ha no problem reading the struct data
        std::cout << "read back: " << obj->string << endl;

        stream.close();
    }

    //obj goes out of scope and throws a read access violation
}

I expect the unique_ptr to just delete the object and no error should be thrown

**********EDIT********************
Thanks for the comments and answers - basically from your help I produced the code that I was trying to do so have listed it here in case it helps anyone else.
Main Points were:
* std::string is not recommended in struct if reading and writing from binary because std::string has unknown number of bytes.
* needed to create the object in memory before assigning the pointer to it - std::make_unique() was suitable for this. 

struct mystruct {
    int val1 = 0;
    double val2 = 5.6;
    char somestring[10] = "string";

};

int main()
{

    //brackets here just to encapsulate the scope for this example
    {
        //try to open the stream for reading
        std::ifstream stream;
        stream.open("test.bin", std::ios::binary | std::ios::in);

        //hold the results in a vector
        auto results = std::vector<std::unique_ptr<mystruct>>();

        //read a vectory or mystructs from the binary file
        while (!stream.eof())
        {
            //create the object - NOTE: make_unique initialises my struct
            std::unique_ptr<mystruct> obj = std::make_unique<mystruct>();

            //read from binary file into obj
            if (!stream.read(reinterpret_cast<char*>(obj.get()), sizeof mystruct))
                break;

            //add the obj to th vector
            results.push_back(std::move(obj));
        }

        stream.close();

        for (auto& val : results)
        {
            cout << "read back: " << val->somestring << endl;
        }


    }
}

Upvotes: 1

Views: 1233

Answers (1)

Nicol Bolas
Nicol Bolas

Reputation: 474236

There are 3 kinds of undefined behavior in your code.

First, you pretend there's an object in a place that there isn't one. You never created a mystruct; you just allocated some bytes of memory. Merely doing a reinterpret_cast isn't sufficient to create a mystruct. So any use of pointer that accesses the "object" that doesn't exist is UB.

Second, even if there was one in that buffer, mystruct is not trivially copyable, so you cannot just copy its bytes around. You can't read a bunch of bytes into a non-trivially copyable object. It is the presence of a non-trivially copyable non-static data member (ie: mystruct::string) that makes it non-trivially copyable.

And third, you try to delete this mystruct. But there there is no mystruct, you're deleting a thing that doesn't exist. Technically, that's probably covered by #1, but it's likely what's causing your code to outright crash.

If you to know why "no problem reading the struct data" happened to work, odds are good that the std::string implementation is using small string optimization, which stores the string inside the std::string itself if it is small enough. For small strings, doing a bytewise copy is probably close enough to "working" to allow you to read the string data.

But that's just getting lucky.

Upvotes: 2

Related Questions