Very strange char array behaviour

Question

.

 unsigned int fname_length = 0;
//fname length equals 30
file.read((char*)&fname_length,sizeof(unsigned int));
//fname contains random data as you would expect
char *fname = new char[fname_length];
//fname contains all the data 30 bytes long as you would expect, plus 18 bytes of      random data on the end (intellisense display)
file.read((char*)fname,fname_length);
//m_material_file (std:string) contains all 48 characters
m_material_file = fname;
// count = 48
int count = m_material_file.length();

now when trying this way, intellisense still shows the 18 bytes of data after setting the char array to all ' ' and I get exactly the same results. even without the file read

char name[30];
for(int i = 0; i < 30; ++i)
{
  name[i] = ' ';
}
file.read((char*)fname,30);
m_material_file = name; 
int count = m_material_file.length();

any idea whats going wrong here, its probably something completely obvious but im stumped!

thanks

Answer 1

Sounds like the string in the file isn't null-terminated, and intellisense is assuming that it is. Or perhaps when you wrote the length of the string (30) into the file, you didn't include the null character in that count. Try adding:

fname[fname_length] = '\0';

after the file.read(). Oh yeah, you'll need to allocate an extra character too:

char * fname = new char[fname_length + 1];

Answer 2

I guess that intellisense is trying to interpret char* as C string and is looking for a '\0' byte.

Answer 3

fname is a char* so both the debugger display and m_material_file = fname will be expecting it to be terminated with a '\0'. You're never explicitly doing that, but it just happens that whatever data follows that memory buffer has a zero byte at some point, so instead of crashing (which is a likely scenario at some point), you get a string that's longer than you expect.

Answer 4

Use

m_material_file.assign(fname, fname + fname_length);

which removes the need for the zero terminator. Also, prefer std::vector to raw arrays.

Answer 5

std::string::operator=(char const*) is expecting a sequence of bytes terminated by a '\0'. You can solve this with any of the following:

1. extend fname by a character and add the '\0' explicitly as others have suggested or
2. use m_material_file.assign(&fname[0], &fname[fname_length]); instead or
3. use repeated calls to file.get(ch) and m_material_file.push_back(ch)

Personally, I would use the last option since it eliminates the explicitly allocated buffer altogether. One fewer explicit new is one fewer chance of leaking memory. The following snippet should do the job:

std::string read_name(std::istream& is) {
    unsigned int name_length;
    std::string file_name;
    if (is.read((char*)&name_length, sizeof(name_length))) {
        for (unsigned int i=0; i<name_length; ++i) {
            char ch;
            if (is.get(ch)) {
                file_name.push_back(ch);
            } else {
                break;
            }
        }
    }
    return file_name;
}

Note:

You probably don't want to use sizeof(unsigned int) to determine how many bytes to write to a binary file. The number of bytes read/written is dependent on the compiler and platform. If you have a maximum length, then use it to determine the specific byte size to write out. If the length is guaranteed to fewer than 255 bytes, then only write a single byte for the length. Then your code will not depend on the byte size of intrinsic types.