Garbage Appended to Output When Reading Values from a File

I'm new to C++ file io, so the other day I decided to write a small program that simply reads a UTF-8 encoded string and a paired float from a binary file. The pattern is string-float with no extra data or spacing between pairs. EDIT I've revised the code based on several answers. However, the output remains the same ("The RoommateAp 0");

string readString (ifstream* file)
{
    //Get the length of the upcoming string
    uint16_t stringSize = 0;
    file->read(reinterpret_cast<char*>(&stringSize), sizeof(char) * 2);

    //Now that we know how long buffer should be, initialize it
    char* buffer = new char[stringSize + 1];
    buffer[stringSize] = '\0';

    //Read in a number of chars equal to stringSize
    file->read(buffer, stringSize);
    //Build a string out of the data
    string result = buffer;

    delete[] buffer;
    return result;
}

float readFloat (ifstream* file)
{
    float buffer = 0;
    file->read(reinterpret_cast<char*>(&buffer), sizeof(float));
    return buffer;
}

int main()
{
    //Create new file that's open for reading
    开发者_运维百科ifstream file("movies.dat", ios::in|ios::binary);
    //Make sure the file is open before starting to read
    if (file.is_open())
    {
        while (!file.eof())
        {
            cout << readString(&file) << endl;
            cout << readFloat(&file)  << endl;
        }
        file.close();
    }
    else
    {
        cout << "Unable to open file" << endl;
    }
}

And a sample of data from the file (spaces for readability):

000C 54686520526F6F6D6D617465 41700000

As one can see, the first two bytes are the length of the string (12 in this case), followed by twelve characters (which spell "The Roommate"), and the final four bytes are a float.

When I run this code, the only thing that happens is that the terminal hangs and I have to close it manually. I think it may be because I am reading past the end of the file, but I have no idea why this would happen. What am I doing incorrectly?

---

There are at least two issues. First, the line:

file->read(reinterpret_cast<char*>(stringSize), sizeof(char) * 2);

Probably should take the address of stringSize:

file->read(reinterpret_cast<char*>(&stringSize), sizeof(stringSize));

Second, the line:

char* buffer = new char[stringSize];

Doesn't allocate enough memory, since it doesn't take the NUL terminator into account. That code should do something like:

//Now that we know how long buffer should be, initialize it
char* buffer = new char[stringSize + 1];
//Read in a number of chars equal to stringSize
file->read(buffer, stringSize);
buffer[stringSize] = '\0';

Finally, the line:

return static_cast<string>(buffer);

Fails to delete[] the buffer after instantiating a string from it, which will cause a memory leak.

Also note that std::string's UTF-8 support is quite poor out of the box. Fortunately, there are solutions.

---

Your code has a few serious problems:

1. file->read(reinterpret_cast<char*>(stringSize), sizeof(char) * 2);

   in this part you are casting the current value of stringSize into a pointer. Your idea was most probably instead to pass the address of the stringSize variable.

2. char* buffer = new char[stringSize];

   This allocates an array of chars, but no one is going to free it. Use std::vector<char> buffer(stringSize); instead so that the memory management will be done correctly for you. To get the address of the buffer you can use &buffer[0].

3. return static_cast<string>(buffer);

   What you probably want is the string constructor that accepts pointers to first and one-past-last characters. In other words: return std::string(&buffer[0], &buffer[0]+stringSize);

---

You cast to a character array. Does the string size number in the file include space for the null character? If not, you might be running off the end of a character array and looping forever on the cout write.

---

There are some issues with your code.

You state that the size of the string is specified by 4 bytes, that means you should use a uint32_t not a uint16_t.

You do not free the memory used when you allocate the string buffer.

string readString(std::ifstream* file)
{
    // Get the length of the upcoming string.
    // The length of the string is specified with 4 bytes: use uint32_t, not uint16_t
    uint32_t stringSize = 0;    
    file->read(reinterpret_cast<char*>(&stringSize), sizeof(uint32_t));

    // Now that we know how long buffer should be, initialize it
    char* buffer = new char[stringSize + 1];
    buffer[stringSize] = '\0'; // null terminate the string

    //Read in a number of chars equal to stringSize
    file->read(buffer, stringSize);
    string result = buffer;

    delete[] buffer;
    return result;
}