0

.

 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

3
  • Purely as a comment: you shouldn't be using sizeof(unsigned int) since it ties your binary file format to the platform - the number of bytes for an unsigned int is a platform specific detail. Commented Oct 22, 2009 at 18:40
  • thanks, what would a better way be? Commented Oct 22, 2009 at 18:43
  • I added a note to my answer about using fixed byte sizes. Commented Oct 22, 2009 at 18:45

5 Answers 5

3

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]; 
Sign up to request clarification or add additional context in comments.

4 Comments

Awesome that seems to have resolved the issue. never seen this happen before in c++. should i be writing the file out as the length of the std:string +1 in order to get the null character at the end or something? as wouldve thought string would have accounted for this file.write((char*)&fname_length,sizeof(unsigned int)); file.write((char*)m_material_file.c_str(),fname_length);
where does 'fname_length' come from when you do that write? If it is just strlen(fname) then it won't include the null terminator character.
sorry, was m_material_file.length(), but I guess as that returns how many actual characters there are then it wouldnt include the null terminating string thanks for your help
Whether you store the nul in the file is up to you. You clearly don't need it since you already store the length in the file. For that reason, I wouldn't also store the nul character. Also, std::string does not consider the nul to be part of its contents, so string::length() will not include it.
1

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

Comments

1

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.

Comments

1

Use

m_material_file.assign(fname, fname + fname_length); 

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

2 Comments

thanks, I do use std::vector for a lot of stuff generally, but im writing a software rasterizer where memory / performance is critical. using raw arrays has given me a very significant boost in performance and also allows me to keep track of my memory better
You should still prefer std::vector<char>. When constructed with the correct argument, it is nothing more than new char[n]. Unlike the latter construct, however, vector will make sure the array is eventually released. Oh, and given the file reads, I very much doubt that the code in question is time-critical. :P
1

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.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.