Read file into vector<byte>

Question

I need to read a range of bytes from a file and return it as a std::vector<std::byte>.

My current code looks like this:

#include <cstdint>
#include <fstream>
#include <string>
#include <vector>

using bytes = std::vector<std::byte>;
using namespace std;

bytes read_block(uint32_t offset,
                 uint32_t length,
                 const string& filename) {
  ifstream is(filename, ios::binary);
  is.seekg(offset);
  byte data[length];
  is.read(reinterpret_cast<char*>(data), length);
  return bytes(data, data + length);
}

But as can be seen I read it into a byte array and then create the vector by copying the bytes in the constructor. I have some issues with this implementation:

• Copying of bytes when creating output variable.
• Use of C style array.
• Use of casts.

I have left out error handling for now to focus on the specific issues above.

Answer 1

Nice:

using bytes = std::vector<std::byte>;

I would call it Bytes to make it clear it is a type rather than an object.

---

This is not a good idea:

using namespace std;

See: Why is “using namespace std;” considered bad practice?

---

This is technically not valid C++.

  byte data[length];

Variable sized arrays (VSA) are an extension to the language supported by several compilers but not actually part of the C++ standard. Though it is part of the more recent versions C standard.

Also creating a local array means that the data is being created on the local stack frame (here I am assuming a Von-Neumann like architecture). This means for the data to exist after the function completes means you will need to copy it out of the function. So an alternative data object is probably a good idea.

Also there is a limitation on the size of array allowed on the stack (there are hardware limitations on the size of stack frames on lots of architecture).

---

So a good idea would be to use the same data structure that you return.

  Bytes data(length);   // This is basically std::vector

Now in older versions of C++ there are some nice optimizations that can be applied to std::vector when it is returned from a function that allow the compiler to build the std::vector at the final destination so that it is not actually copied on return.

In modern versions of the compiler the language has the concept of move semantics built into the language. So returning a std::vector is not an expensive operation as the internal buffer is moved (just the pointer is copied) from the function to the destination result.

Also the compiler is now required to perform copy elision of objects when it can.

---

Error checking

This code does zero error checking. There are several things that can go wrong and you don't check for any of them. But you said you removed this (you should have left it in).

---

So a better version:

#include <vector>
#include <string>
#include <fstream>
#include <cstdint>

using Bytes = std::vector<std::byte>;

Bytes read_block(std::uint32_t      offset,
                 std::uint32_t      length,
                 std::string const& filename)
{
  // Not going to explicitly check these.
  // The use of gcount() below will compensate for a failure here.
  std::ifstream is(filename, std::ios::binary);
  is.seekg(offset);

  Bytes data(length);
  is.read(reinterpret_cast<char*>(data.data()), length);

  // We have to check that reading from the stream actually worked.
  // If any of the stream operation above failed then `gcount()`
  // will return zero indicating that zero data was read from the
  // stream.
  data.resize(is.gcount());

  // Simply return the vector to allow move semantics take over.
  return data;
}

Answer 2

Pass a std::istream& instead of a filename

Your function has two responsibilities: opening the file and reading it. I would let it only read from an already opened file. This means the function can be shorter and have less responsibilities, and at the same time flexibility is increased because now it can read from any std::istream, not just from a std::ifstream. So:

bytes read_block(..., std::istream &is) {
    is.seekg(offset);
    ...
}

This way, your function will also be able to work on a std::stringstream, and in the future with std::spanstream.

Note that this is similar to how std::getline() works.

Answer 3

I played around with this a bit more and came up with some alternatives.

First I tried to use basic_ifstream<byte> like this:

bytes read_block(uint32_t offset,
                 uint32_t length,
                 const string& filename) {
  basic_ifstream<byte> is(filename, ios::binary);
  istreambuf_iterator<byte> it(is);
  bytes data;
  copy_n(next(it, offset), length, data.begin());
  return data;
}

this compiles without issues and does not use C style arrays nor casts and copies directly from the file to the output vector (thus solving all of my mentioned issues).

However this throws a std::bad_cast at runtime. Based on this it might be due to stl not having a char_traits for std::byte. Annoying that it's a runtime issue and not a compile issue. As of now I don't know how or if it's simple/possible to implement a char_traits for std::byte myself.

My next version loosens up on the cast restriction, but works also at runtime:

bytes read_block(uint32_t offset,
                 uint32_t length,
                 const string& filename) {
  ifstream is(filename, ios::binary);
  is.seekg(offset);
  bytes data;
  data.resize(length);
  is.read(reinterpret_cast<char*>(data.data()), length);
  return data;
}

Answer 4

Let's look at the signature of the function:

bytes read_block(uint32_t offset,
                 uint32_t length,
                 const string& filename)

Instead of using the (optional) std::uint32_t we should accept the type we actually need for seekg() and read(), namely std::ifstream::pos_type and std::streamsize.

Consider reordering the arguments so that users get the benefit of default values. I'd suggest filename first (as that's non-defaultable), then offset (default to beginning of file), then length (default to the whole file):

bytes read_block(const std::string& filename,
                 std::ifstream::pos_type offset = {},
                 std::streamsize length = std::numeric_limits<std::streamsize>::max())