std::vector writing/reading back gives garbagge

Question

I am trying to write a vector of class "Product" in to a file and read it back. However I get garbage loaded while reading. Can somebody review what could be possibly going wrong? Or suggest an alternate method.

#include <fstream>
#include <vector>
#include <iostream>

class Product
{
  public:
      std::string name;
      int code;
      double price;
};


int
main ()
{
  const char *const file_name = "products.bin";
  {
    std::vector < Product > prod
    {
      Product {"Honey", 1, 12.34}, 
      Product {"CoffeeBeans", 2, 56.78}, 
      Product {"Cl", 3, 90.12}
    };

    for (const Product & p:prod)
       std::cout << p.name << ' ' << p.code << ' ' << p.price << '\n';

    std::ofstream file (file_name, std::ios::binary);
    size_t sz = prod.size ();

    file.write (reinterpret_cast < const char *>(&sz), sizeof (sz));
    file.write (reinterpret_cast < const char *>(&prod[0]), sz * sizeof (prod[0]));
  }

  {
    std::vector < Product > prod;
    std::ifstream file (file_name, std::ios::binary);

    size_t sz;
    file.read (reinterpret_cast < char *>(&sz), sizeof (sz));
    prod.resize (sz);
    file.read (reinterpret_cast < char *>(&prod[0]), sz * sizeof (prod[0]));

    for (const Product & p:prod)
         std::cout << p.name << ' ' << p.code << ' ' << p.price << '\n';
  }

}

> Blockquote

Stop bin-dumping non-[POD](http://stackoverflow.com/questions/146452/what-are-pod-types-in-c) types to files. `Product` contains a `std::string` member. That isn't going gently in to , and especially out of, that file. — WhozCraig, Feb 24 '15 at 00:15
How can you both write the above code, and not realize `std::string` cannot be bin-dumped? ... Did you copy some of that code from someone else by any chance? — Yakk - Adam Nevraumont, Feb 24 '15 at 00:29
`reinterpret_cast` means your code is wrong unless you specifically know why it is actually right. — Neil Kirk, Feb 24 '15 at 00:32
More explicitly, `std::string` will store the text in dynamically allocated memory (unless it implements optional "Short String Optimisation", then strings of a few characters may be stored internally). So, you're writing out pointers to the file that aren't still valid after the original vector storing the `Product`s has left the first scope and been destroyed - all those `std::string`s were `delete`d and their dynamically allocated memory released. — Tony Delroy, Feb 24 '15 at 00:34
Maybe consider not using binary for this and read/write your values in text format using something like `file << p.name << '\n' << p.code << '\n' << p.price << '\n'`? — Galik, Feb 24 '15 at 01:09

score 2 · Answer 1 · answered Feb 24 '15 at 00:32

The standard library member std::ostream::write writes raw data to the output stream. As a condition of that write, said-data must be trivially copyable in nature.

Your Product is no-such type. It contains a std::string member. std::string is (usually) implemented with a dynamic buffer managed by a pointer internal to the object. Thus it is not a trivially copyable type, and thus Product is not a trivially copyable type.

If you're dead-set on storing this data in raw bytes, a protocol is required, one which both the reader and the writer agree on. How complex this protocol becomes is entirely up to you and how diverse your end-needs are, and since you're the author of both sides, those decisions are entirely up to you. If the target file should be multi-platform/endian independent, things can get complex very quickly. If the target is same-platform-only, you can skip some otherwise tedious nuances in developing your protocol.

Personally, I'd opt for a simple delimited text-line-per record entry. The space you think you're otherwise saving by performing this in bin-format would likely be negligible, and it is near-trivial to develop both the reader and the writer. As a bonus, it is highly likely you will have platform and endian independence free of charge.

Best of luck.

Even if an object is *trivially copyable* it may contain raw pointers to unowned memory, or even to somewhere within the same object, and thought should be given to whether such pointers will be meaningful when the objects are reloaded, whether by the same or another process. — Tony Delroy, Feb 24 '15 at 00:51

dtech · Answer 2 · 2015-02-24T01:02:20.387

For writing and reading non-POD types you really should write your own serialization and deserialization function for each such object, which writes all of the object's members in a defined order and also reads them in that order. Also, you should keep endianness in mind if you want the data to be portable.

It is preferable to use a serialization functions for POD types as well. For a few reasons:

different compilers/platforms might have some data types in different size
different compilers/platforms might arrange the members in a different order
both of the above can result in extra padding

There are also the potential pointers in the object or its members or the vtable, stuff you don't need nor should you serialize.

All of those will be problematic if you use raw data.

score 1 · Answer 3 · answered Feb 24 '15 at 00:37

The value of non-POD types (before C++11) or non-trivially-copyable types (C++11) does not survive binary copy operations. This includes using memcpy() to create a copy, or (as you have done) writing to a file in binary mode and reading back. Such operations (memcpy(), binary read and write) only have defined meanings for POD types.

std::string is not such a type, since (among other things) it has user-defined constructors, assignment operators, and destructor that manage a resource (memory). Any stuct/class type that contains an instance of a non-copyable type is also not a copyable type.

score 0 · Answer 4 · answered Feb 24 '15 at 00:31

You can't write binary, std::string have dynamic memory.

You should write member a member, as text by example:

file<<prod[0].name<<' '<<prod[0].code<<' '<<prod[0].price<<' '; //You must separate diferent fields with spaces

And read in the same way:

file>>prod[0].name>>prod[0].code>>prod[0].price

std::vector writing/reading back gives garbagge

4 Answers4