Sign in to follow this  
NukeCorr

odd C++ bug(?)

Recommended Posts

Okay, I have no idea what's happening here.
...
#define DEFAULT_KILLSMSG 6
...
int killsMsgWep[DEFAULT_KILLSMSG];
...
// init()
// this for() loop, crashes the program
// but if I replace -1 with 0 or something else, it does not crash
for(int i=0;i<DEFAULT_KILLSMSG;i++)
{
   killsMsgWep[i] = -1;
}
...
// then i decided to check which of them crashes it
killsMsgWep[0] = -1;
killsMsgWep[1] = -1; // <--- this one, when it's -1, it crashes the program
killsMsgWep[2] = -1;
killsMsgWep[3] = -1;
killsMsgWep[4] = -1;
killsMsgWep[5] = -1;
...

Those are the only places "killsMsgWep" variable is used, nowhere else. So how can the initializion crash the program with such simple variable? Am I missing something here or what's this about :S

Share this post


Link to post
Share on other sites
Quote:
Original post by NukeCorr
Okay, I have no idea what's happening here.

*** Source Snippet Removed ***
Those are the only places "killsMsgWep" variable is used, nowhere else.
So how can the initializion crash the program with such simple variable?
Am I missing something here or what's this about :S


That works completely fine when run in isolation, what compiler are you using? I'm guessing there must be something else in your code causing this, perhaps memory has been corrupted somewhere else in your code.

Share this post


Link to post
Share on other sites
I'm using MSVC++ 6.0 (yes I know I should get the latest one).

If it's corrupted memory, how do I fix or prevent that, and what might be causing it?

EDIT: the program has over 40 files of source, could that affect in it too?

Share this post


Link to post
Share on other sites
Quote:
Original post by grekster
Is that array defined globally like your example indicates, or is it actually part of a class or something in your actual code?


it's part of a class under public:

Share this post


Link to post
Share on other sites
Quote:
Original post by NukeCorr
Quote:
Original post by grekster
Is that array defined globally like your example indicates, or is it actually part of a class or something in your actual code?


it's part of a class under public:


And the init() function is too? In that case is the instance of the class you're calling init() on valid? if not then that could easily cause a crash.

If init() isn't part of the class, what instance are you accessing the array on?

Share this post


Link to post
Share on other sites
init() is in the same class as the array, and the class has many other defined variables and functions (bool, int, etc). And the others work just fine, but this new array I added causes failure

Share this post


Link to post
Share on other sites
Yes but is the actual instance of the class you are calling init() on valid?

Also it may be helpful to show some more actual code, like the whole of the init function plus where it gets called from.

Share this post


Link to post
Share on other sites
Sorry, I don't understand your question..

anyway, I removed the array just to test does the rest of the code work and now it doesn't, it did earlier though.
When I run debugger it gives me this:
http://img17.imageshack.us/img17/2447/acsv.png

Share this post


Link to post
Share on other sites
It seems to me like the class instance you are actually calling init on is invalid, possibly a null pointer or just uninitialized e.g.:

MyClass myInstance;
myInstance.init(); // <-- good, valid instance of the class!

MyClass* myInstance;
myInstance->init(); // <---- bad! uninitialized pointer, not a real instance of 'MyClass'

Quote:
Also it may be helpful to show some more actual code, like the whole of the init function plus where it gets called from.

Share this post


Link to post
Share on other sites
Okay here are some parts of the code which are used in the init(), they don't use all the advantages of C++ and has some C language stuff in it too.

Are there some critical errors/mistakes in them?


void CGame::AddKillsMessage(int killer, int victim, int weapon)
{
char *temp;
temp = new char[128];
strcpy(temp, Player[killer].name);
strcat(temp, " killed ");
strcat(temp, Player[victim].name);
/*strcat(temp, " with ");
strcat(temp, ProjInfo[weapon].name);*/


killsMsg[0] = temp;
//killsMsgWep[0] = weapon;

int i;

for(i=DEFAULT_KILLSMSG;i>0;i--)
{
if(i > 0)
{
killsMsg[i] = killsMsg[i-1];
//killsMsgWep[i] = killsMsgWep[i-1];
}
}

delete[] temp;
}

...

void CGame::LoadBot(char *bot)
{
char szPath[255];
sprintf(szPath, "bots/%s.bot", bot);

char *botName = "";
int aiLev = 1;
int ignHits = 2;

char* szResult = new char[255];
memset(szResult, 0x00, 255);

GetPrivateProfileString(bot, "Name", "", szResult, 255, szPath);
botName = szResult;
aiLev = GetPrivateProfileInt(bot, "Difficulty", 1, szPath);
ignHits = GetPrivateProfileInt(bot, "IgnoreHits", 2, szPath);
if(ignHits < 0 || ignHits > 255) ignHits = 2;

for(int i=0;i<MAX_PLAYERS;i++)
if(Player[i].active && i != player)
if(Player[i].name == "Tank")
{
Player[i].name = botName;
if(aiLevel > AI_HARD)
Player[i].SetDifficulty(aiLev);
Player[i].ignoreHits = ignHits;

delete[] szResult;

return;
}
}

...

void CTileMap::New(int s_x, int s_y)
{
TILES_X = s_x;
TILES_Y = s_y;

int c, i;

Tiles = new int*[TILES_X];
for(i=0;i<TILES_X;i++)
Tiles[i] = new int[TILES_Y];

for(c=0;c<TILES_Y;c++)
for(i=0;i<TILES_X;i++)
{
Tiles[i][c] = TILE_PLATEFLOOR;

if(rand() % 8 == 2)
Tiles[i][c] = TILE_STONEWALL;
}
}

...

void CGame::StartNewMatch()
{
int i;

for(i=0;i<MAX_PLAYERS;i++)
Player[i].Remove();
for(i=0;i<MAX_PROJ;i++)
Proj[i].Remove();
for(i=0;i<MAX_PART;i++)
Part[i].Remove();
for(i=0;i<MAX_BONUS;i++)
Bonus[i].Remove();

dispPlayer = 0;
numOfPlayers = 0;

showStats = false;
for(i=0;i<TEAM_MAX;i++)
{
if(i==TEAM_RED)
teamName[i] = "RED TEAM";
else if(i==TEAM_BLUE)
teamName[i] = "BLUE TEAM";
else if(i==TEAM_GREEN)
teamName[i] = "GREEN TEAM";
else if(i==TEAM_YELLOW)
teamName[i] = "YELLOW TEAM";

teamScore[i] = 0;
}

msg = "";
msgTimer = 0;
msgType = 0;

roundDone = false;
winner = -1;
endDelay = DEFAULT_ENDDELAY;

lastPlayer = 0;
lastProp = 0;

showMainMenu = false;
showGameMenu = false;
mainMenuView = 0;

for(i=0;i<DEFAULT_KILLSMSG;i++)
{
killsMsg[i] = "";
//killsMsgWep[i] = -1;
}

Map.New(50,50);

AddRandomBonuses();

if(!editMode)
{
LoadMap("maps/edit_map.tbm");

if(gameMode == GAMEMODE_DM)
{
for(i=0;i<addBots;i++)
{
AddPlayer(1,1,true,TEAM_NONE);
Player[lastPlayer].SetDifficulty(aiLevel);
}
}
else if(gameMode == GAMEMODE_TDM)
{
for(i=0;i<addBots;i++)
{
if(i % 2 == 0)
AddPlayer(1,1,true,TEAM_RED);
else
AddPlayer(1,1,true,TEAM_BLUE);

Player[lastPlayer].SetDifficulty(aiLevel);
}
}

LoadBot("Bob");
LoadBot("i pwn j00 n4p");
LoadBot("HACKER");
LoadBot("assbag");
LoadBot("noob");
LoadBot("1337 !!");
LoadBot("BEP");
LoadBot("D.O.G");
LoadBot("Yourself");
LoadBot("Ich bin angst!");
LoadBot("Reptile");
LoadBot("a tank behind you");

if(player > -1)
Player[player].name = playerName;
}
}
...

Share this post


Link to post
Share on other sites
What grekster is hinting at is this:

struct Foo {
void bar() { a = 42; }

int a;
};

void main() {
Foo * foo = 0;
foo->bar(); // Will crash! foo instance is not valid

foo = new Foo();
foo->bar(); // Will work
delete foo;

foo->bar() // Will maybe crash, maybe work! Undefined behavior, foo instance is not valid
}


So the question is, is the instance of the class holding the killsMsgWep array valid?

Share this post


Link to post
Share on other sites
Quote:
Original post by Promethium
What grekster is hinting at is this:
*** Source Snippet Removed ***
So the question is, is the instance of the class holding the killsMsgWep array valid?


Yes it is valid

Share this post


Link to post
Share on other sites
Quote:
Original post by NukeCorr
Quote:
Original post by Promethium
What grekster is hinting at is this:
*** Source Snippet Removed ***
So the question is, is the instance of the class holding the killsMsgWep array valid?


Yes it is valid


Are you sure? how did you prove it was valid? Can we see the code where init() is called?

Share this post


Link to post
Share on other sites
InitGame() is the init() and it's in CGame class

void CGame::StartGame()
{
int systemState, gameState;

isRunning = true;

ResetGameSettings();
LoadConfig("config.ini");

systemState = Core.InitSystem();

gameState = InitGame();

if(systemState < 0)
isRunning = false;

if(gameState < 0)
isRunning = false;

GameLoop();

SaveConfig("config.ini");

QuitGame();
}

...

int CGame::InitGame()
{
...
...
...
mx = 0;
my = 0;
clicked = 0;

viewX = 0;
viewY = 0;

CHEAT = false;
DEBUG = false;
editMode = false;
if(editMode)
SDL_ShowCursor(1);
editTile = 0;
newTilesX = 50;
newTilesY = 50;

StartNewMatch();

showMainMenu = true;
showGameMenu = false;
mainMenuView = 0;
menuRotator = 0;

return 1;
}


Share this post


Link to post
Share on other sites
Have you tried something like this to test it:


#define DEFAULT_KILLSMSG 6
...
int* killsMsgWep;
...

void CGame::AddKillsMessage(int killer, int victim, int weapon)
{
if(!killsMsgWep)
{
killsMsgWep = new int[DEFAULT_KILLSMSG];
}

char *temp;
temp = new char[128];
strcpy(temp, Player[killer].name);
strcat(temp, " killed ");
strcat(temp, Player[victim].name);
/*strcat(temp, " with ");
strcat(temp, ProjInfo[weapon].name);*/


killsMsg[0] = temp;
//killsMsgWep[0] = weapon;

int i;

for(i=DEFAULT_KILLSMSG;i>0;i--)
{
if(i > 0)
{
killsMsg[i] = killsMsg[i-1];
//killsMsgWep[i] = killsMsgWep[i-1];
}
}

delete[] temp;
}



Of course that is just an example as it would probably be better to add it in the StartNewMatch() function, but you will get the idea. Pull it out of the Init() function and try this.

Also, does this happen in both debug and release builds?

Share this post


Link to post
Share on other sites
You could also try this as well:

int killsMsgWep[DEFAULT_KILLSMSG] = { -1, -1, -1, -1, -1, -1 };

Of course this takes away from the DEFAULT_KILLSMSG definition as it could be variable, but it's alright to test it this way.

Share this post


Link to post
Share on other sites
Quote:
Original post by mrbastard
You still haven't shown where you create your instance of the CGame class.


Hmm, this?


// main.cpp

#pragma once
#include "preheader.h"
#include "game.h"

CGame Game;

int main(int argc, char **argv)
{
Game.StartGame();

return 0;
}





EDIT:

I got it working now with the method UltimaX suggested... I guess I'll go with that then..

anyway, here's a screenshot if someone's thinking what kind of game it is...

http://img246.imageshack.us/img246/7098/80236669.png


[Edited by - NukeCorr on March 5, 2010 7:44:53 AM]

Share this post


Link to post
Share on other sites
Quote:
Original post by NukeCorr
Are there some critical errors/mistakes in them?

Yes. That code will invoke undefined behaviour.
	killsMsg[0] = temp;

This is wrong, because you delete temp at the end of the scope (although using new[]/delete[] is bad practice here, using an automatic array will give the same error). Change that line to this.
	killsMsg[0] = strdup(temp);

But ya know, if you used C++ and std::string instead of 1960's-era C this sort of thing wouldn't happen.

Edit: fixed strdup() function name.

[Edited by - Bregma on March 5, 2010 11:42:31 AM]

Share this post


Link to post
Share on other sites
strcpy takes 2 parameters, and i dont see where killsMsg is defined but it looks like it's used as a char * in the rest of the code. There is all sorts of wrong goin on in this code!!

Is this what was meant?

strcpy(killsMsg,temp);

or is killsMsg really a char **?

Share this post


Link to post
Share on other sites
Quote:
Original post by Atrix256
strcpy takes 2 parameters, and i dont see where killsMsg is defined but it looks like it's used as a char * in the rest of the code. There is all sorts of wrong goin on in this code!!

Is this what was meant?

strcpy(killsMsg,temp);

or is killsMsg really a char **?


Yea I know it has lot of mistakes although they seem to work ingame, killsMsg is:
std::string killsMsg[DEFAULT_KILLSMSG]; not char

Share this post


Link to post
Share on other sites
Quote:
Original post by NukeCorr
killsMsg is: std::string killsMsg[DEFAULT_KILLSMSG]; not char

If killsMsg is an array of std:strings, why not simply concatenate it as an std::string?
killsMsg[0] = Player[killer].name + " killed " + Player[victim].name + " with " + ProjInfo[weapon].name
Assuming that all those name properties are std::strings too of course.

Share this post


Link to post
Share on other sites
Quote:
Original post by Wan
Quote:
Original post by NukeCorr
killsMsg is: std::string killsMsg[DEFAULT_KILLSMSG]; not char

If killsMsg is an array of std:strings, why not simply concatenate it as an std::string?
killsMsg[0] = Player[killer].name + " killed " + Player[victim].name + " with " + ProjInfo[weapon].name
Assuming that all those name properties are std::strings too of course.


You got a point... I used C# many years back and it's stuck in my head.

Thanks for that one

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