[Freeciv-Dev] Re: (PR#2638) better assert checking for autogames

["Jason Short via RT" <rt@xxxxxxxxxxxxxx>]

On Mon, 2002-12-23 at 17:29, Per I. Mathisen via RT wrote:
> 
> This patch adds some better assert checking for autogames, and also
> improves the logging done in get_defender() + adds an assert there.
> 
> These changes were vital for me to track down various bugs. Liberally
> sprinkling CHECK_UNIT() around in a function suspected of not detecting
> unit death properly is a very nice way of locating the problem.
> 
> CHECK_UNIT() is only available in games compiled with --with-debug=yes

> Index: ai/aitools.h
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/ai/aitools.h,v
> retrieving revision 1.31
> diff -u -r1.31 aitools.h
> --- ai/aitools.h      2002/12/18 19:05:21     1.31
> +++ ai/aitools.h      2002/12/23 22:21:59
> @@ -22,6 +22,16 @@
>  struct government;
>  struct player;
>  
> +#ifdef DEBUG
> +#define CHECK_UNIT(punit)                                        \
> +  assert(punit);                                                 \
> +  assert(punit->type < U_LAST);                                  \
> +  assert(punit->owner < MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS);   \
> +  assert(find_unit_by_id(punit->id));
> +#else
> +#define CHECK_UNIT(punit) /* check punit here when in debug mode */
> +#endif
> +
>  enum bodyguard_enum {
>    BODYGUARD_WANTED=-1,
>    BODYGUARD_NONE

To make this function-like macro into a usable expression, change it to
something like:

#ifdef DEBUG
#define CHECK_UNIT(punit)        \
  (assert(punit),                \
   assert(punit->type < U_LAST), \
   assert(punit->owner < MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS), \
   assert(find_unit_by_id(punit->id)))
#else
#define CHECK_UNIT(punit) 0
#endif

The idea is the macro should work correctly even if used in weird
situations like

 if (foo)
   CHECK_UNIT(punit);
 else
   CHECK_UNIT(punit2);

even though such situations should, in general, not arise given our
coding style.

In other places do { ... } while(0) is used to achieve the same thing;
however, in this case it is easy just to make the macro an expression.

Note that your current macro can give "empty statement" warnings even
when used normally, since when expanded you will get unnecessary
semicolons.

jason