Complete.Org: Mailing Lists: Archives: freeciv-dev: August 2005:
[Freeciv-Dev] (PR#13651) bug in 13613 commit
Home

[Freeciv-Dev] (PR#13651) bug in 13613 commit

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
Subject: [Freeciv-Dev] (PR#13651) bug in 13613 commit
From: "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Thu, 11 Aug 2005 09:28:00 -0700
Reply-to: bugs@xxxxxxxxxxx

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=13651 >

Per Inge Mathisen wrote:
> On Thu, 11 Aug 2005, Raimar Falke wrote:
> 
>>>+case S_HUT:
>>>+case S_FORTRESS:
>>>+case S_SPECIAL_2:
>>>+case S_RIVER:
>>>+case S_FARMLAND:
>>>+case S_AIRBASE:
>>>+case S_FALLOUT:
>>>+case S_LAST:
>>>+  break;
>>
>>I think there should be a default branch with an assert.
> 
> 
> I prefer it like this, because gcc will give a warning when there is a new
> enum that the switch does not catch. It is much easier to find missing
> treatments of a new enum this way rather than rely on eyeballs/grep or to
> run the game until it asserts (if it ever does during testing).

I agree entirely.  An assert is easily possible, but shouldn't be done 
with a default: case.  (In this case there's already an implicit assert 
since an out-of-range special will cause a crash in tile_set|clear_special.)

This patch fixes the recursion, which was obviously an error.  This kind 
of confusion could strike again but I don't see what to do about it.

-jason

Index: common/tile.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/tile.c,v
retrieving revision 1.11
diff -p -u -r1.11 tile.c
--- common/tile.c       11 Aug 2005 04:51:09 -0000      1.11
+++ common/tile.c       11 Aug 2005 16:24:55 -0000
@@ -243,7 +243,7 @@ void tile_change_terrain(struct tile *pt
 ****************************************************************************/
 void tile_add_special(struct tile *ptile, enum tile_special_type special)
 {
-  tile_add_special(ptile, special);
+  tile_set_special(ptile, special);
 
   switch (special) {
   case S_FARMLAND:
@@ -286,7 +286,7 @@ void tile_add_special(struct tile *ptile
 ****************************************************************************/
 void tile_remove_special(struct tile *ptile, enum tile_special_type special)
 {
-  tile_remove_special(ptile, special);
+  tile_clear_special(ptile, special);
 
   switch (special) {
   case S_IRRIGATION:

[Prev in Thread] Current Thread [Next in Thread]
  • [Freeciv-Dev] (PR#13651) bug in 13613 commit, Jason Short <=