bugfixing trouble again..

[will_asher]

A crash bug was reported in DaJAngband recently and I've narrowed it down to a certain part of the earthquake function (a part that I added to the function), but I can't figure out why it's making it crash. It's not a nessesary part of the function so I can just comment it out to fix it, but that seems like a cop out and I'd rather find out why it's making it crash and fix it.
any help?

/* Examine the quaked region */
for (dy = -r; dy <= r; dy++)
{
for (dx = -r; dx <= r; dx++)
  {
         s16b this_o_idx, next_o_idx = 0;
         int qbreak;
         /* Extract the location */
         yy = cy + dy;
         xx = cx + dx;

         /* Skip unaffected grids */
         if (!map[16+yy-cy][16+xx-cx])
         {

	/* some fragile objects get destroyed even in unaffected grids */
	for (this_o_idx = cave_o_idx[yy][xx]; this_o_idx; this_o_idx = next_o_idx)
	{
		/* get the object */
		object_type *o_ptr = &o_list[this_o_idx];

		/* Get the next object */
		next_o_idx = o_ptr->next_o_idx; /* <<< */
/* this is the line the debugger points to when it crashes */

		/* get odds for object to break (TRUE makes things much less likely to break because it's not a destroyed grid) */
		qbreak = quake_break(o_ptr, TRUE);
		if (artifact_p(o_ptr)) qbreak = 0;

		/* roll for destruction */
		if (rand_int(100) < qbreak)
		{
			/* message */
			if (player_can_see_bold(yy, xx))
			{
				char o_name[80];
				object_desc(o_name, sizeof(o_name), o_ptr, FALSE, 0);
				msg_format("The %s breaks!", o_name);
			}
			delete_object_idx(this_o_idx);
		}
	}

(spacing changed to hopefully make it more readable in the post)


(PS: not sure whether I should post this in the variants section or the development section, but I figured development was for V stuff.)

[Pete Mack]

object_type *o_ptr = &o_list[this_o_idx];


You are relying on this_o_idx being non-zero, when instead you should be relying on o_ptr being non-null. (!)

[zaimoni]

object_type *o_ptr = &o_list[this_o_idx];


You are relying on this_o_idx being non-zero, when instead you should be relying on o_ptr being non-null. (!)

o_ptr will be non-NULL, as long as o_list is non-NULL (which is handled in initialization). (This code obviously was swiped from V's project_o, which uses the exact same construct).

What it won't automatically be, is valid: that requires verifying that

(0<this_o_idx) && (this_o_idx<z_info->o_max)

If I recall correctly, the debug mode in recent MSVC can detect out of bounds array accesses (unreliably). Zaiband V3.0.10 has been hardened against this sort of error (all writes to m_idx or o_idx type fields are range-checked in asserts).

[will_asher]

Zaimoni's right. The FOR loop was copied from another place in the code. There are very similar FOR loops in several places in the code which all start with the same line:

for (this_o_idx = cave_o_idx[yy][xx]; this_o_idx; this_o_idx = next_o_idx)

I just added this line:

if (!((0 < this_o_idx) && (this_o_idx < z_info->o_max))) break;

right before this line:

/* Get the next object */
next_o_idx = o_ptr->next_o_idx;

and it didn't crash the next time. thanks Zaimoni!