Sign in to follow this  
suliman

Wierd access violation (c++)

Recommended Posts

suliman    1653
Hi
I cannot understand why i get an access violation (unhandled exeption) from my code.

I call
loadMap("test.map");

And loadmap function is like this:
[code]void world::loadMap(char * name){

char dest[40];
sprintf(dest,"data/maps/%s",name);
FILE * f = fopen(dest,"rb");
if(f){
fileLoad2(city);
gui.message("map loaded");

removeExtension(name);

sprintf(tempText,"data/maps/%s.jpg",name);
if(fileExist(tempText))
gfx.tempPicSetup(&tempTexture,tempSprite,tempText);

}
else
gui.message("ERROR load");
if(f)
fclose(f);
}[/code]

It crashes on removeExtension
[code]void removeExtension( char * string )
{
for(int i=0;i<80;i++){
if(string[i]=='.'){
string[i]=0;
break;
}
}
}[/code]

On the line where i set string[i]=0;
This function works fine in other situations, so i dont know why it crashes on my here...

Plz help, thanks
Erik

Share this post


Link to post
Share on other sites
Wooh    1088
A string literal will give a const char array that can't be changed. That is why you get an exception when you try to change it. Your compiler should warn you about this though.

Share this post


Link to post
Share on other sites
suliman    1653
1. But sometimes the exact same procedure works fine. How is that possible? It should still be a const char array in those cases.
2. What is the proper way to write the function call (parameters must be changed i guess)

Thanks for your help
Erik

Share this post


Link to post
Share on other sites
Wooh    1088
1. It is undefined behavior so anything can happen.
2. Calling the function as loadMap("test.map") gives me a warning "warning: deprecated conversion from string constant to `char*`". Maybe you can turn on some extra warnings to make it easier to detect such mistakes. What you want to do is change the parameter to `const char * name` and change the function so it works.

Since you are using C++ and not C I think you should look into the C++ Standard Library and use that instead. That will make things much easier and look something like this (untested):[code]std::string removeExtension(const std::string& str)
{
std::size_t pos = str.find_last_of('.');
if (pos != std::string::npos) {
return str.substr(0, pos);
}
return str;
}


void world::loadMap(const std::string& name) {
std::ifstream f("data/maps/" + name, std::ios::in | std::ios::binary);
if (f) {
fileLoad2(city);
gui.message("map loaded");

std::string tempText = "data/maps/" + removeExtension(name) + ".jpg";
if (fileExist(tempText))
gfx.tempPicSetup(&tempTexture,tempSprite,tempText);
}
else
gui.message("ERROR load");
}[/code]

Share this post


Link to post
Share on other sites
Slavik81    360
Writing to that memory is not guaranteed to crash your program. It's just not guaranteed to not crash your program.

You probably don't want to have the input be a editable string anyways. It would be really confusing when a function called loadMap changes the name that you give it.

If you want to stick with the c-syle code, you'd just change loadMap to take a const char* name, and use strcpy to make a new name buffer that you can freely change as you need or desire.


A more C++-style way of doing it would be to use std::string for name, which will do the copy for you.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this