BIG problem. How do I find a bug in so much code?

Okay, I know it's not a buffer overrun. I check for that everytime I add to an array.

They look like this

Code:

---

// Declaration
#define MAX_WHATEVERS 100
WHATEVER_STRUCT whatever[MAX_WHATEVERS];
int whatevers = 0;

// When making a whatever
  if(whatevers + 2 > MAX_WHATEVERS)
      return FALSE;

---

Yes, that's even got a name: Heisenbug - based on the "Heisenberg uncertainty theory", that says [paraphrazed] "The closer you get to something, the more likely it is to not behave normally", and in code, "The more debugging you do, the more likely it is that the code behaves differently".

Utterly annoying, but true.

Having different levels of debug output, so that you can turn on debugging only on some functions is often key here - if you have some sort of idea of where problem resides that is. There are two ways [which can be combined] of doing such things:
1. A bitmask, where you enable/disable groups of logging. So if you set bit X, then you get logging for component X, if you set bit Y, you get logging for component Y.
2. A level - the more level you give, the more output you get.

Using a standard function to provide ALL of the logging will help tremendously here.

By the way, this looks a bit suspect:

Code:

---

DestroyTorpedo(18)
TorpedoImpact(18)
TorpedoImpact(19)
TorpedoImpact(20)
TorpedoImpact(21)

---

I don't have a clue what your app does, but it sounds like you are destroying stuff before you are finished with it - may be that the function names are a bit wrong here and it does something else...


--
Mats

Mastadex, yes, the SAFE_DELETE sounds good, I do need to look at that. But I use arrays to prevent having to deal with pointers. When I delete somthing, I'm really making everything beyond it's point in the array, fall back on it by one. Then bring down the amount varible by one.

Quote:

---

I prefer using the SAFE_DELETE() macro, its a no-brainer when it comes to deleting things. It sets the pointer to NULL afterwards, which protects against double deletes. You might want to look into that, IMO.

---

Instead of using a macro, use a function template. Stroustrup gives an example of a destroy() function template that does just what you described for SAFE_DELETE in his answer to Why doesn't delete zero out its operand?

Of course, this is assuming you have to work with raw pointers in the first place instead of working with smart pointers and containers to benefit from RAII.

My gut feeling says the ai is where the program is going haywire, but I have no way I knowing that. I'll have to do that with the logging components/levels.

matsp, Here is my DestroyTorpedo function:

Code:

---

void DestroyTorpedo(int one)
{
  log("DestroyTorpedo(%d)", one); // Added for debugging purposes
  for(e = one; e < torps; e++)
      torp[e] = torp[e + 1];
  torps--;
  return;
}

---

as you can see, DestroyTorpedo makes torpedo 19 become torpedo 18, so that isn't a problem.

>> Using a standard function to provide ALL of the logging will help tremendously here.
I don't know if I follow. But I'm currently calling log(), and thats the only function I call to record this stuff.

Here is my log function

Code:

---

// dolog is a boolean that I can activate during runtime by pressing L.

void log(const char *fmt, ...)
{
  if(fmt == NULL || !dolog) return;
  char text[MAX_PATH];
  va_list ap;
  va_start(ap, fmt);
  vsprintf(text, fmt, ap);
  va_end(ap);               
  FILE *fp = fopen("log.txt", "a+");
  fprintf(fp, "%s\r\n", text);
  fclose(fp);
  return;
}

---

do you know of any way of increasing it's preformence?

good advice there, laserlight, thanks.

Ok, so if you add a "level" and/or perhaps a "bitmask" to your "log" function, you can restrict the logging by level and/or bitmask, so you only get overview logging.

Also, using just fflush, rather than opening and closing the file every time will speed the logging up by a fair bit.

--
Mats

How about actually using std::vector + std::vector.at and smart pointers? You can narrow it down a bit further, perhaps even eliminate the problem.
I didn't have time to reply, but I guess you saw the result. Logging and flushing a lot is quite expensive.

Quote:

---

Code:

---

void DestroyTorpedo(int one)
{
  log("DestroyTorpedo(%d)", one); // Added for debugging purposes
  for(e = one; e < torps; e++)
      torp[e] = torp[e + 1];
  torps--;
  return;
}

---

---

Off-by-one error. Condition should be "e < torps-1", or the last iteration will access "torp[torps]", and that's probably an invalid location.
If you use a fixed-size array for torpedoes, that could explain the apparently random crashes: it would only crash when you completely fill the torpedo array.

Quote:

---

Originally Posted by Yarin

Code:

---

void DestroyTorpedo(int one)
{
  log("DestroyTorpedo(%d)", one); // Added for debugging purposes
  for(e = one; e < torps; e++)
      torp[e] = torp[e + 1];
  torps--;
  return;
}

---

---

This loop overruns the torp[] array by a single slot. It iterates until and including e = torps - 1, so the last copy is:

Code:

---

torp[torps - 1] = torp[torps]

---

torp[torps] is off the end of the array. This is a read violation but still plenty capable of causing a crash.

EDIT: Doh. That's what I get for posting in an old window. CB got it.

Hmm, I never noticed that! Thanks for pointing that out, I didn't realize I've been using bad code in so many programs. (I've been handling game objects in this way for a while now.)

EDIT:
But is that still really a problem when considering this code?

Code:

---

BOOL ShootTorpedo(int org, int dest, BOOL bForward)
{
  log(CREATE, "ShootTorpedo(%d, %d, %s)", org, dest, bForward ? "TRUE" : "FALSE");
  if(torps + 2 > MAX_TORPS)
      return FALSE;
  torp[torps].x = ship[org].x;
  torp[torps].y = ship[org].y;
  torp[torps].r = ship[org].r;
  if(bForward) torp[torps].r += HALFRAD;
  torp[torps].f = 0;
  torp[torps].t = dest;
  torp[torps].o = org;
  torps++;
  PlaySound(14);
  return TRUE;
}

---

As you can see, the array is never even allowed to get completely full to begin with.

The only way is to try it!
Is there a reason you don't use vectors and smart pointers?

Code:

---

BOOL ShootTorpedo(int org, int dest, BOOL bForward)
{
  log(CREATE, "ShootTorpedo(%d, %d, %s)", org, dest, bForward ? "TRUE" : "FALSE");
  if(torps + 2 > MAX_TORPS)
      return FALSE;
  torp[torps].x = ship[org].x;
  torp[torps].y = ship[org].y;
  torp[torps].r = ship[org].r;
  if(bForward) torp[torps].r += HALFRAD;
  torp[torps].f = 0;
  torp[torps].t = dest;
  torp[torps].o = org;
  torps++;
  PlaySound(14);
  return TRUE;
}

---

Don't you still make torps equal to MAX_TORPS and thus cause the off-by-one error?

Okay, here are the individual types of code logging that can be turned on and off now.

Code:

---

#define DESTROY 0 // When things get destroyed
#define CREATE  1 // When things get created
#define ALTER  2 // When the condition of somting changes
#define THINK  3 // When the AI decides what to do
#define BASIC  4 // When basic root actions happen

---

I've still yet to find out what's causing the problem. (I'm thinking there may be more than one now. :()
The only time the logger was running when it crashed, the function BlowShip(1) was last called.

Here it is:

Code:

---

BOOL BlowShip(int one)
{
  log(CREATE, "BlowShip(%d)", one);
  BOOL blowshiptemp = FALSE;
  if(booms + 2 < MAX_SHIPS)
  {
      boom[booms].x = ship[one].x;
      boom[booms].y = ship[one].y;
      boom[booms].r = -ship[one].r;
      boom[booms].s = (shipsizex[ship[one].t] + shipsizey[ship[one].t]) / 120.0;
      boom[booms].f = 0;
      booms++;
      blowshiptemp = TRUE;
  }
  DestroyShip(one);
  PlaySound(3);
  return blowshiptemp;
}

---

As you can see, I do check for a possible buffer overrun.

Okay, now I've set all my arrays to have 2 more than the max, so an overrun is out of the question, right?

Code:

---

TORP torp[MAX_TORPS + 2];

---

>> Is there a reason you don't use vectors and smart pointers?
What advantage do I gain by using them?