Reading text from a file

[ICantC]

I have a text adventure game in ANSI C that is trying to load a text file containing the area description from a .txt file into a heap allocated buffer, which is then copied into the area structs char* description variable.

while the below code executes OK, when I come to print the contents of area->description to the screen, it crashes with:

__fastfail(FAST_FAIL_STACK_COOKIE_CHECK_FAILURE);

please can someone advise what may be wrong? I'm using 5000 bytes below just as an example, the file itself contains about 5 lines of text. (Visual Studio 2019, Windows 10)

void area_load_description(area_t *area)
{
	FILE *fp = fopen("myFile", "r");

	if (fp == ((void *)0))
	{
		m_error("FAILED TO LOAD AREA DESCRIPTION FILE");
		return;
	}

	char *buffer = malloc(5000); //more than enough storage for the file

	int ch;
	int i = 0;
	while ((ch = fgetc(fp)) != EOF)
	{
		buffer[i] = ch;
		i++;
	}

        buffer[i] = '\0';
	area->description = malloc(5000);
	strcpy(area->description, buffer);

	free(buffer);
	fclose(fp);
}

[Ganado]

You should show a minimal version of your code that still reproduces the issue; otherwise, we are just guessing.

Some things to check:
- Are you sure i never goes above 4999?
- Is the memory for what area points to properly allocated?

[AbstractionAnon]

I see the same two things that Ganado pointed out.
- You don't limit check i in your while loop.
- Since you're using malloc, you don't check for an error return from malloc.
Using the VS debugger, set a break point at line 21. Verify the value of i.
Set another breakpoint at line 23. Verify the value of area->description.

[jonnin]

just use arrays for small strings that you destroy after a bit.
why do you copy it at all? Why not allocate and dump it directly into area->description when you read it in? buffer serves no purpose?

[seeplus]

Note that if the file contains 5000 bytes, then you are using 5001 bytes of memory - an extra one to store the terminating null.

[kbw]

You can get the file's size with stat. It uses the file metadata, and doesn't touch the file. Often, folk use fseek/ftell, but don't do that. Use that size (+ 1) as the buffer size.

You can then use fread to read the whole file into the buffer as you now know the size.

No need for the second malloc, you just need:

area->description = buffer;

[seeplus]

Note that stat isn't standard c/c++. On Windows systems its _stat().
See https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions?view=msvc-160

[JLBorges]

std::filesystem::file_size is standard C++.
https://en.cppreference.com/w/cpp/filesystem/file_size
But the posted code seems to be pure C.

[oggin]

The c file read/write functions are of course second to none.
I am tempted to use them, but recently I switched to the C++ methods.
Example:

#include <iostream>
#include <sstream>
#include <fstream>

using namespace std;


bool fileexists(const char *fileName)
{
    ifstream infile(fileName);
    return infile.good();
}

int filelen(const char* filename)
{
    ifstream in(filename,ifstream::ate | ifstream::binary);
    return in.tellg(); 
}

void load(const char* filename,string &content)
{
	ifstream file(filename);
    stringstream strStream;
    strStream << file.rdbuf(); 
    content = strStream.str();
    file.close();
}

void save(const char* filename,string content)
{		
		ofstream file(filename);
		file << content;
	    file.close();
}


int main()
{
string s,ret;
int i;
for (i=1;i<101;i++) // create a string to save
{
	if (i<10)
	{s+="0"+to_string(i)+"  ";}
	else
	{s+=to_string(i)+"  ";}
	
	if (i % 10 == 0) {s+="\n";}
}
cout<<"To file:"<<endl;
cout<<s<<endl;
save("stringtester.txt",s);
cout<<"From file:"<<endl;
load("stringtester.txt",ret);
cout<<ret<<endl<<endl;
cout<<"characters in file "<<filelen("stringtester.txt")<<endl;

		std::remove("stringtester.txt");
	
	if (fileexists("stringtester.txt")) {cout<<"Delete the file manually"<<endl;}
	else
	{ cout<<"The file has been deleted"<<endl;}

		cout <<"Press return to end . . ."<<endl; 
	 cin.get();
		 	
}

 

[ICantC]

Thank you very much for the replies all, I've done some refactoring based on your comments to improve it :)

My apologies, but I wrongly assumed the posted code was the problem, when it wasn't. It turned out the "stack cookie check failure" occured when I tried to read/print area->description to the screen. But the code handling that was also correct. I replaced all 5 lines of text contained in the file with just "aaaaa" etc, and it worked fine. So the error seems to be caused by some formatting in the text file for some reason

[George P]

I wrongly assumed

Been there, done that, likely to do it again in the future.

That is why the 'puter Deities invented debuggers, so us mere mortals can step through our compiled code and discover where the evilness actually is.

[jonnin]

also, consider testing small parts at a time. first, hard code a couple of strings and see if your bool function works. Then add applying it to a file, which you first read and validate that it read the file by printing to screen. Then apply the function to the data ... not much can go wrong now that you have 2 working pieces, but debugging 2 non working pieces is much harder...