1

Yes, I have this kind of problems because it was always hard for me to follow the pointers operations. So, I have this simple code:

struct myfile {
    char* name;
    char* content;
    long size;
};

myfile this_file;

int main() {
    read();
    return 0;
}

void read() {
    output("Please, specify file name: ");
    cin >> (this_file.name = new char);
    FILE *stream;
    stream = fopen(code.name, "r");
    if (stream != NULL) {
        fseek(stream , 0, SEEK_END);
        myfile.size = ftell(codefile);
        myfile.content = new char[myfile.size];
        fseek(myfile, 0, SEEK_SET);
        if ((fread(myfile.content, 1, myfile.size, stream)) == 0) {
            fclose(codefile);
            cout << "File is empty!\n");
        }
    }
 }

It gets the name of the file correctly, it gets the size of the content but when trying to allocate space to the content member the program crashes, which I know is some pointers issue, but, as always, can't remember/figure out what is it. It crashes when reaching this line: myfile.content = new char[myfile.size];

I need your help again. Thanks!

ali
  • 10,569
  • 20
  • 77
  • 128

3 Answers3

6

This is causing a buffer overrun:

cin >> (this_file.name = new char);

as the operator>>() will consume characters up the next whitespace character and write to this_file.name which only has enough space for a single character.

Use std::string (instead of char*) with std::getline():

std::string name;
if (std::getline(cin, name))
{
    // Use 'name'.
}

If you are investigating pointers you need to allocate an array of char and prevent reading beyond the allocated array:

this_file.name = new char[32];
if (cin.getline(this_file.name, 32))
{
}

Remember to delete and delete[] what is new or new[].

There is an alternative for reading the content of a file into std::string using streams. See What is the best way to read an entire file into a std::string in C++?.

Community
  • 1
  • 1
hmjd
  • 113,589
  • 17
  • 194
  • 245
3

To be clear this is telling you about the pointer problem, you should not use pointers like this in any production code, use string or vector, it's what thay are for!

This line :- cin >> (this_file.name = new char); Is a little.... strange

While it's legal to do this, any experienced c++ programming looking at this is going to be suprised.

Plus you only allocate one single character which will be sufficient to store a terminating null character in but no actual text.

Do something like this -

this_file.name = new char[100];
cin >> this_file.name;

As others have said though, don't do this in real life except for learning, you have to guess how big the input will be and if the user sends more it will break things. All those security bugs that you read about in browsers and so on? This is how they happen! :)

There seem to be other issues with this code such as creating a variable called this_file and then apparently expecting it to be called "code" on the next part so it needs a lot more fixing that this pointer problem.

Also, remember to "delete[] this_file.name" to free the memory at the end. Better still use std::unique_ptr instead if you really want to use your own memory allocation here.

jcoder
  • 28,098
  • 16
  • 76
  • 120
  • 1
    Yes, I'm just learning C++. If I would have to create some useful application I would take use of any library I can and save a lot of time and problems. Thanks! – ali Nov 28 '12 at 15:23
0

Use std::string instead of char* and std::ifstream instead of FILE*, and all the problems will be solved.

user1610015
  • 6,364
  • 2
  • 12
  • 17