# Reading all bytes from a file

I'm basically trying to write a helper function that reads a whole file and returns the data and the number of bytes read.

Can you tell me if is correctly written and used?

#include <iostream>

static char * ReadAllBytes(const char * filename, int * read)
{
ifstream ifs(filename, ios::binary|ios::ate);
ifstream::pos_type pos = ifs.tellg();
int length = pos;
char *pChars = new char[length];
ifs.seekg(0, ios::beg);
ifs.read(pChars, length);
ifs.close();
*read = length;
return pChars;
}

int _tmain(int argc, _TCHAR* argv[])
{
const char * filename = "polar00.map";
int read ;
char * pChars = ReadAllBytes(filename, &read);
delete[] pChars;
return 0;
}

-

## 1 Answer

A few things I would do differently:

static char * ReadAllBytes(const char * filename, int * read)
{
ifstream ifs(filename, ios::binary|ios::ate);
ifstream::pos_type pos = ifs.tellg();

// What happens if the OS supports really big files.
// It may be larger than 32 bits?
// This will silently truncate the value/
int length = pos;

// Manuall memory management.
// Not a good idea use a container/.
char *pChars = new char[length];
ifs.seekg(0, ios::beg);
ifs.read(pChars, length);

// No need to manually close.
// When the stream goes out of scope it will close the file
// automatically. Unless you are checking the close for errors
// let the destructor do it.
ifs.close();
*read = length;
return pChars;
}


How I would do it:

static std::vector<char> ReadAllBytes(char const* filename)
{
ifstream ifs(filename, ios::binary|ios::ate);
ifstream::pos_type pos = ifs.tellg();

std::vector<char>  result(pos);

ifs.seekg(0, ios::beg);
ifs.read(&result[0], pos);

return result;
}


Note:

static std::vector<char> ReadAllBytes(char const* filename)


It may seem like an expensive copy operation. But in reality NRVO will make this an in-place operation so no copy will take place (just make sure you turn on optimizations). Alternatively pass it as a parameter:

static void ReadAllBytes(char const* filename, std::vector<char>& result)

-
I love this answer, never thought of referencing vector[0] to access the raw memory block underneath it. – Jason Larke Feb 20 '13 at 7:53
This is very typical when using C++ containers and calling C code. – Loki Astari Feb 20 '13 at 16:20
I typically used the new unsigned char[size] approach, which meant I had to ensure the cleanup afterwards. This is much nicer. – Jason Larke Feb 21 '13 at 3:06
Thank you, it's much simpler and cleaner. I thought of using array<char> but since it is (from what I've read) allocated on the stack it was not a good idea as my data wouldn't necessarily be small. Well, I've forgot to apply this again : "premature optimization is the root of all evil" ... – Aybe Feb 21 '13 at 12:12
An alternative to &vector[0] is vector.data(). – Aidiakapi Jul 18 '15 at 17:38