Reputation: 11045
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!
Upvotes: 1
Views: 814
Reputation: 30035
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.
Upvotes: 3
Reputation: 121971
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++?.
Upvotes: 6
Reputation: 6678
Use std::string instead of char* and std::ifstream instead of FILE*, and all the problems will be solved.
Upvotes: 0