[CF-Devel] Some bug fixes
crossfire-devel at archives.real-time.com
crossfire-devel at archives.real-time.com
Mon Mar 1 01:44:09 CST 2004
Hi,
I'd like to present some bug fixes for the server. (I'll commit them if
there are no objections.) The patches correct some out of bounds
accesses to arrays that occurred on my local server.
For the first patch (patch-1.diff), I do not have any specific item to
reproduce the bug but it occurred frequently that "enc > 20".
The loops in patch-2.diff run one too far, especially for (client) map
size 25x25: in this case the loops run from -1 to 25 (inclusive).
The patch patch-3.diff fixes two out of range errors in lines 737+ and
1061+. The remaining changes replace the sizeof-expressions with the
macro "arraysize". Is this change acceptable or should I correct the
bugs only? Maybe the macro should go into a header file (which one?)
because there are some other places (server/commands.c,
server/alchemy.c, ...) where it could be used.
patch-4.diff fixes some invalid array references when no free spot can
be found. (Only the first one in treasure.c did actually crash.) I don't
think, the fixes for the exit (and maybe the key) are very good because
they drop the exit or place the key in an invalid place.
The bug in patch-5.diff did happen in /pup_land/nurnberg/dick/bomb1 with
the first titan (it has Str=60). For me, this patch is not yet ready for
inclusion because similar problems exist for dex_bonus[] and maybe other
xxx_bonus[]. My preferred solution is to create new functions to
calculate the bonus. These functions can take care for the correct index
values.
Andreas
-------------- next part --------------
Index: common/item.c
===================================================================
RCS file: /cvsroot/crossfire/crossfire/common/item.c,v
retrieving revision 1.39
diff -u -w -r1.39 item.c
--- common/item.c 18 Dec 2003 20:39:44 -0000 1.39
+++ common/item.c 27 Feb 2004 19:56:46 -0000
@@ -205,16 +205,7 @@
if(QUERY_FLAG(op,FLAG_SEE_IN_DARK)) enc += 1;
if(QUERY_FLAG(op,FLAG_MAKE_INVIS)) enc += 1;
-#if 0
- if (enc > 20) {
- LOG(llevDebug,"calc_item_power got %d enchantments for %s\n", enc, op->name?op->name:"(null)");
- enc = 20;
- }
-#endif
- /* Items only have a positive power rating */
- if (enc < 0) enc = 0;
-
- return enc_to_item_power[enc];
+ return get_power_from_ench(enc);
}
-------------- next part --------------
Index: common/los.c
===================================================================
RCS file: /cvsroot/crossfire/crossfire/common/los.c,v
retrieving revision 1.10
diff -u -w -r1.10 los.c
--- common/los.c 2 Apr 2003 08:12:53 -0000 1.10
+++ common/los.c 27 Feb 2004 19:56:46 -0000
@@ -441,6 +441,6 @@
* be blocked by different spaces in front, this mean that a lot of spaces
* could be examined multile times, as each path would be looked at.
*/
- for (x=(MAP_CLIENT_X - op->contr->socket.mapx)/2 - 1; x<(MAP_CLIENT_X + op->contr->socket.mapx)/2 + 1; x++)
- for (y=(MAP_CLIENT_Y - op->contr->socket.mapy)/2 - 1; y<(MAP_CLIENT_Y + op->contr->socket.mapy)/2 + 1; y++)
+ for (x=(MAP_CLIENT_X - op->contr->socket.mapx)/2; x<(MAP_CLIENT_X + op->contr->socket.mapx)/2; x++)
+ for (y=(MAP_CLIENT_Y - op->contr->socket.mapy)/2; y<(MAP_CLIENT_Y + op->contr->socket.mapy)/2; y++)
check_wall(op, x, y);
-------------- next part --------------
Index: common/readable.c
===================================================================
RCS file: /cvsroot/crossfire/crossfire/common/readable.c,v
retrieving revision 1.13
diff -u -w -r1.13 readable.c
--- common/readable.c 13 Sep 2003 05:01:30 -0000 1.13
+++ common/readable.c 27 Feb 2004 19:56:47 -0000
@@ -78,6 +78,9 @@
* are not needed anyplace else, so why have them globally declared?
*/
+/* Calculate the number of items in an array variable. */
+#define arraysize(array) (sizeof(array)/sizeof(*(array)))
+
/* 'title' and 'titlelist' are used by the readable code */
typedef struct titlestruct {
char *name; /* the name of the book */
@@ -450,1 +453,1 @@
};
-static int max_titles[6] =
+static int max_titles[] =
{
- ((sizeof (light_book_name) / sizeof (char *)) + (sizeof (heavy_book_name) / sizeof (char *))) * (sizeof (book_author) / sizeof (char *)),
- (sizeof (mon_book_name) / sizeof (char *)) * (sizeof (mon_author) / sizeof (char *)),
- (sizeof (art_book_name) / sizeof (char *)) * (sizeof (art_author) / sizeof (char *)),
- (sizeof (path_book_name) / sizeof (char *)) * (sizeof (path_author) / sizeof (char *)),
- (sizeof (formula_book_name) / sizeof (char *)) * (sizeof (formula_author) / sizeof (char *)),
- (sizeof (gods_book_name) / sizeof (char *)) * (sizeof (gods_author) / sizeof (char *))
+ (arraysize(light_book_name) + arraysize(heavy_book_name)) * arraysize(book_author),
+ arraysize(mon_book_name) * arraysize(mon_author),
+ arraysize(art_book_name) * arraysize(art_author),
+ arraysize(path_book_name) * arraysize(path_author),
+ arraysize(formula_book_name) * arraysize(formula_author),
+ arraysize(gods_book_name) * arraysize(gods_author)
};
/******************************************************************************
@@ -737,7 +740,7 @@
}
LOG (llevDebug, " book archives(used/avail): ");
bl = booklist;
- while (bl && max_titles[i])
+ while (bl && i < arraysize(max_titles) && max_titles[i])
{
LOG (llevDebug, "(%d/%d)", bl->number, max_titles[i]);
bl = bl->next;
@@ -860,5 +863,4 @@
static void
new_text_name (object *book, int msgtype)
{
- int nbr;
char name[MAX_BUF];
if (book->type != BOOK)
@@ -869,36 +871,29 @@
switch (msgtype)
{
case 1: /*monster */
- nbr = sizeof (mon_book_name) / sizeof (char *);
- strcpy (name, mon_book_name[RANDOM () % nbr]);
+ strcpy (name, mon_book_name[RANDOM () % arraysize(mon_book_name)]);
break;
case 2: /*artifact */
- nbr = sizeof (art_book_name) / sizeof (char *);
- strcpy (name, art_book_name[RANDOM () % nbr]);
+ strcpy (name, art_book_name[RANDOM () % arraysize(art_book_name)]);
break;
case 3: /*spellpath */
- nbr = sizeof (path_book_name) / sizeof (char *);
- strcpy (name, path_book_name[RANDOM () % nbr]);
+ strcpy (name, path_book_name[RANDOM () % arraysize(path_book_name)]);
break;
case 4: /*alchemy */
- nbr = sizeof (formula_book_name) / sizeof (char *);
- strcpy (name, formula_book_name[RANDOM () % nbr]);
+ strcpy (name, formula_book_name[RANDOM () % arraysize(formula_book_name)]);
break;
case 5: /*gods */
- nbr = sizeof (gods_book_name) / sizeof (char *);
- strcpy (name, gods_book_name[RANDOM () % nbr]);
+ strcpy (name, gods_book_name[RANDOM () % arraysize(gods_book_name)]);
break;
case 6: /*msg file */
default:
if (book->weight > 2000)
{ /* based on weight */
- nbr = sizeof (heavy_book_name) / sizeof (char *);
- strcpy (name, heavy_book_name[RANDOM () % nbr]);
+ strcpy (name, heavy_book_name[RANDOM () % arraysize(heavy_book_name)]);
}
- else if (book->weight < 2001)
+ else
{
- nbr = sizeof (light_book_name) / sizeof (char *);
- strcpy (name, light_book_name[RANDOM () % nbr]);
+ strcpy (name, light_book_name[RANDOM () % arraysize(light_book_name)]);
}
break;
}
@@ -915,4 +910,3 @@
add_author (object *op, int msgtype)
{
char title[MAX_BUF], name[MAX_BUF];
- int nbr = sizeof (book_author) / sizeof (char *);
if (msgtype < 0 || strlen (op->msg) < 5)
return;
@@ -923,26 +917,21 @@
switch (msgtype)
{
case 1: /* monster */
- nbr = sizeof (mon_author) / sizeof (char *);
- strcpy (name, mon_author[RANDOM () % nbr]);
+ strcpy (name, mon_author[RANDOM () % arraysize(mon_author)]);
break;
case 2: /* artifacts */
- nbr = sizeof (art_author) / sizeof (char *);
- strcpy (name, art_author[RANDOM () % nbr]);
+ strcpy (name, art_author[RANDOM () % arraysize(art_author)]);
break;
case 3: /* spellpath */
- nbr = sizeof (path_author) / sizeof (char *);
- strcpy (name, path_author[RANDOM () % nbr]);
+ strcpy (name, path_author[RANDOM () % arraysize(path_author)]);
break;
case 4: /* alchemy */
- nbr = sizeof (formula_author) / sizeof (char *);
- strcpy (name, formula_author[RANDOM () % nbr]);
+ strcpy (name, formula_author[RANDOM () % arraysize(formula_author)]);
break;
case 5: /* gods */
- nbr = sizeof (gods_author) / sizeof (char *);
- strcpy (name, gods_author[RANDOM () % nbr]);
+ strcpy (name, gods_author[RANDOM () % arraysize(gods_author)]);
break;
case 6: /* msg file */
default:
- strcpy (name, book_author[RANDOM () % nbr]);
+ strcpy (name, book_author[RANDOM () % arraysize(book_author)]);
}
sprintf (title, "of %s", name);
@@ -1024,8 +1013,6 @@
void
change_book (object *book, int msgtype)
{
- int nbr = sizeof (book_descrpt) / sizeof (char *);
-
switch (book->type)
{
case BOOK:
@@ -1061,5 +1048,5 @@
/* Don't have any default title, so lets make up a new one */
else
{
- int numb, maxnames = max_titles[msgtype];
+ int numb, maxnames = max_titles[msgtype >= 0 && msgtype < arraysize(max_titles) ? msgtype : arraysize(max_titles)-1];
char old_title[MAX_BUF], old_name[MAX_BUF];
if (book->title)
@@ -1121,7 +1108,7 @@
book->title = add_string (old_title);
/* Lets give the book a description to individualize it some */
if (RANDOM () % 4)
- sprintf (old_name, "%s %s", book_descrpt[RANDOM () % nbr], old_name);
+ sprintf (old_name, "%s %s", book_descrpt[RANDOM () % arraysize(book_descrpt)], old_name);
book->name = add_string (old_name);
}
else if (book->title && strlen (book->msg) > 5) /* archive if long msg texts */
@@ -1336,7 +1323,7 @@
*/
i=0;
do {
- index = RANDOM () % (sizeof (art_name_array) / sizeof (arttypename));
+ index = RANDOM () % arraysize(art_name_array);
type = art_name_array[index].type;
al = find_artifactlist (type);
i++;
@@ -1581,14 +1568,14 @@
* water of section.
*/
sprintf(title,"%s: %s of %s",
- formula_book_name[RANDOM() % (sizeof(formula_book_name) / sizeof(char*))],
+ formula_book_name[RANDOM() % arraysize(formula_book_name)],
op_name, formula->title);
}
else
{
sprintf (retbuf, "%sThe %s", retbuf, op_name);
sprintf(title,"%s: %s",
- formula_book_name[RANDOM() % (sizeof(formula_book_name) / sizeof(char*))],
+ formula_book_name[RANDOM() % arraysize(formula_book_name)],
op_name);
if (at->clone.title)
{
-------------- next part --------------
Index: random_maps/exit.c
===================================================================
RCS file: /cvsroot/crossfire/crossfire/random_maps/exit.c,v
retrieving revision 1.16
diff -u -w -r1.16 exit.c
--- random_maps/exit.c 7 Oct 2001 06:45:40 -0000 1.16
+++ random_maps/exit.c 27 Feb 2004 19:56:48 -0000
@@ -245,6 +245,7 @@
if(the_exit_down) {
char buf[2048];
i = find_first_free_spot(the_exit_down->arch,map,downx,downy);
+ if(i==-1) return;
the_exit_down->x = downx + freearr_x[i];
the_exit_down->y = downy + freearr_y[i];
RP->origin_x = the_exit_down->x;
Index: random_maps/treasure.c
===================================================================
RCS file: /cvsroot/crossfire/crossfire/random_maps/treasure.c,v
retrieving revision 1.19
diff -u -w -r1.19 treasure.c
--- random_maps/treasure.c 8 Jan 2003 08:39:18 -0000 1.19
+++ random_maps/treasure.c 27 Feb 2004 19:56:48 -0000
@@ -176,6 +176,7 @@
/* first, find a place to put the chest. */
i = find_first_free_spot(find_archetype("chest"),map,x,y);
+ if(i==-1) return 0;
xl = x + freearr_x[i]; yl = y + freearr_y[i];
/* if the placement is blocked, return a fail. */
@@ -305,6 +306,7 @@
/* if we don't find a good keymaster, drop the key on the ground. */
if(the_keymaster==NULL) {
int freeindex = find_first_free_spot(the_key->arch,map,i,j);
+ if(freeindex==-1) freeindex = 0;
kx = i + freearr_x[freeindex];
ky = j + freearr_y[freeindex];
}
-------------- next part --------------
Index: server/player.c
===================================================================
RCS file: /cvsroot/crossfire/crossfire/server/player.c,v
retrieving revision 1.145
diff -u -w -r1.145 player.c
--- server/player.c 11 Feb 2004 08:09:29 -0000 1.145
+++ server/player.c 27 Feb 2004 19:56:49 -0000
@@ -1455,6 +1455,7 @@
object *left, *bow;
tag_t left_tag, tag;
int bowspeed;
+ sint8 str;
if (!dir) {
new_draw_info(NDI_UNIQUE, 0, op, "You can't shoot yourself!");
@@ -1545,2 +1546,2 @@
* added to the damage. I think the strength bonus is more proper.
*/
+ str = op->stats.Str;
+ if(str < 0) str = 0;
+ if(str > MAX_STAT) str = MAX_STAT;
arrow->stats.dam += (QUERY_FLAG(bow, FLAG_NO_STRENGTH) ?
- 0 : dam_bonus[op->stats.Str]) +
+ 0 : dam_bonus[str]) +
bow->stats.dam + bow->magic + arrow->magic;
/* update the speed */
arrow->speed = (float)((QUERY_FLAG(bow, FLAG_NO_STRENGTH) ?
- 0 : dam_bonus[op->stats.Str]) +
+ 0 : dam_bonus[str]) +
bow->magic + arrow->magic) / 5.0 +
(float)bow->stats.dam / 7.0;
-------------- next part --------------
_______________________________________________
crossfire-devel mailing list
crossfire-devel at lists.real-time.com
https://mailman.real-time.com/mailman/listinfo/crossfire-devel
More information about the crossfire
mailing list