[Freeciv-Dev] [PATCH] min/max -> MIN/MAX

[Markus Linnala <maage@xxxxxxxxx>]

This patch fits as general code cleanup category.

Remove extra min/max macros from common/city.h as we have
perfectly good ones at common/shared.h. Change min and max to
uppercase.

I don't trust compiler can optimize out complext MIN/MAX macros.
When function call is not inlineable, compiler definitely does
not optimize as good as human.

1999-06-28  Markus Linnala  <maage@xxxxxxxxx>

        * common/city.h: remove min/max macros

        * server/cityturn.c (citizen_happy_units): don't trust compiler
        can optimize out complex MIN/MAX macros or function calls, change
        min -> MIN, max -> MAX
        (unhappy_city_check):
        (set_pollution):
        (city_support):
        (city_incite_cost): 
        * server/unittools.c (unit_bribe_cost):
        * ai/advdomestic.c (ai_eval_buildings): 
        * server/citytools.c (city_corruption):
        * server/cityturn.c (citizen_happy_size):
        * ai/aitools.c (ai_gold_reserve):

diff -u --recursive --new-file --exclude-from=.diffignore 
freeciv-cvs/ai/advdomestic.c freeciv-minmax/ai/advdomestic.c
--- freeciv-cvs/ai/advdomestic.c        Sat Jun 12 15:03:19 1999
+++ freeciv-minmax/ai/advdomestic.c     Sun Jun 27 22:28:23 1999
@@ -224,9 +224,11 @@
     if (city_happy(pcity) && pcity->size > asz-1 && est_food > 0)
       values[B_AQUEDUCT] = ((((city_got_effect(pcity, B_GRANARY) ? 3 : 2) *
          pcity->size * game.foodbox)>>1) - pcity->food_stock) * food;
-    else values[B_AQUEDUCT] = food * est_food * asz * game.foodbox /
-           MAX(1, ((asz+1 - MIN(asz, pcity->size)) * MAX(asz, pcity->size) *
-           game.foodbox - pcity->food_stock));
+    else {
+      int l = ((asz+1 - MIN(asz, pcity->size)) * MAX(asz, pcity->size) *
+              game.foodbox - pcity->food_stock);
+      values[B_AQUEDUCT] = food * est_food * asz * game.foodbox / MAX(1, l);
+    }
   }
 
 
@@ -348,9 +350,11 @@
     if (city_happy(pcity) && pcity->size > ssz-1 && est_food > 0)
       values[B_SEWER] = ((((city_got_effect(pcity, B_GRANARY) ? 3 : 2) *
          pcity->size * game.foodbox)>>1) - pcity->food_stock) * food;
-    else values[B_SEWER] = food * est_food * ssz * game.foodbox /
-          MAX(1, ((ssz+1 - MIN(ssz, pcity->size)) * MAX(ssz, pcity->size) *
-          game.foodbox - pcity->food_stock));
+    else {
+      int l = ((ssz+1 - MIN(ssz, pcity->size)) * MAX(ssz, pcity->size) *
+              game.foodbox - pcity->food_stock);
+      values[B_SEWER] = food * est_food * ssz * game.foodbox / MAX(1, l);
+    }
   }
 
   if (could_build_improvement(pcity, B_STOCK))
@@ -414,8 +418,10 @@
       if (i == B_LEONARDO) {
         unit_list_iterate(pcity->units_supported, punit)
           j = can_upgrade_unittype(pplayer, punit->type);
-          if (j >= 0) values[i] = MAX(values[i], 8 * 
unit_upgrade_price(pplayer,
-               punit->type, j)); /* this is probably wrong -- Syela */
+         if (j >= 0) {
+           int l = 8 * unit_upgrade_price(pplayer, punit->type, j);
+           values[i] = MAX(values[i], l); /* this is probably wrong -- Syela */
+         }
         unit_list_iterate_end;
       }
       if (i == B_BACH)
diff -u --recursive --new-file --exclude-from=.diffignore 
freeciv-cvs/ai/aitools.c freeciv-minmax/ai/aitools.c
--- freeciv-cvs/ai/aitools.c    Sat Jun 12 15:03:20 1999
+++ freeciv-minmax/ai/aitools.c Sun Jun 27 22:29:21 1999
@@ -79,7 +79,8 @@
 **************************************************************************/
 int ai_gold_reserve(struct player *pplayer)
 {
-  return MAX(pplayer->ai.maxbuycost, total_player_citizens(pplayer)*2);
+  int i = total_player_citizens(pplayer)*2;
+  return MAX(pplayer->ai.maxbuycost, i);
 /* I still don't trust this function -- Syela */
 }
 
diff -u --recursive --new-file --exclude-from=.diffignore 
freeciv-cvs/common/city.h freeciv-minmax/common/city.h
--- freeciv-cvs/common/city.h   Sat Jun 12 15:03:27 1999
+++ freeciv-minmax/common/city.h        Sun Jun 27 22:38:43 1999
@@ -79,8 +79,6 @@
 /* for new city: default auto-attack options all on, others off: */
 #define CITYOPT_DEFAULT (CITYOPT_AUTOATTACK_BITS)
 
-#define min(X, Y) ((X)>(Y) ? (Y) : (X))
-#define max(X, Y) ((X)<(Y) ? (Y) : (X))
 #define get_government(X) (game.players[X].government)
 
 #define CITY_MAP_SIZE 5
diff -u --recursive --new-file --exclude-from=.diffignore 
freeciv-cvs/server/citytools.c freeciv-minmax/server/citytools.c
--- freeciv-cvs/server/citytools.c      Sat Jun 12 15:03:29 1999
+++ freeciv-minmax/server/citytools.c   Mon Jun 28 20:43:08 1999
@@ -449,8 +449,10 @@
     capital=find_palace(city_owner(pcity));
     if (!capital)
       dist=36;
-    else
-      dist=min(36,map_distance(capital->x, capital->y, pcity->x, pcity->y));
+    else {
+      int tmp = map_distance(capital->x, capital->y, pcity->x, pcity->y);
+      dist=MIN(36,tmp);
+    }
   }
   if (get_government(pcity->owner) == G_DESPOTISM)
     dist = dist*2 + 3; /* yes, DESPOTISM is even worse than ANARCHY */
diff -u --recursive --new-file --exclude-from=.diffignore 
freeciv-cvs/server/cityturn.c freeciv-minmax/server/cityturn.c
--- freeciv-cvs/server/cityturn.c       Sat Jun 12 15:03:30 1999
+++ freeciv-minmax/server/cityturn.c    Mon Jun 28 20:43:38 1999
@@ -147,10 +147,13 @@
 **************************************************************************/
 void citizen_happy_size(struct city *pcity)
 {
-  int citizens;
-  citizens = min(pcity->size, content_citizens(&game.players[pcity->owner])); 
-  pcity->ppl_content[0] = max(0, (citizens - city_specialists(pcity)));
-  pcity->ppl_unhappy[0] = max(0, (pcity->size - (pcity->ppl_content[0] + 
city_specialists(pcity))));
+  int citizens, tmp;
+  tmp = content_citizens(&game.players[pcity->owner]);
+  citizens = MIN(pcity->size, tmp); 
+  tmp = (citizens - city_specialists(pcity));
+  pcity->ppl_content[0] = MAX(0, tmp);
+  tmp = (pcity->size - (pcity->ppl_content[0] + city_specialists(pcity)));
+  pcity->ppl_unhappy[0] = MAX(0, tmp);
   pcity->ppl_happy[0]=0;
 }
 
@@ -195,12 +198,12 @@
       unhap--;
   }
   if (unhap>0) {                                                           
-    step=min(unhap,pcity->ppl_content[3]);                          
+    step=MIN(unhap,pcity->ppl_content[3]);                          
     pcity->ppl_content[3]-=step;
     pcity->ppl_unhappy[3]+=step;
     unhap-=step;                     
     if (unhap>0) {                       
-      step=min((unhap/2),pcity->ppl_happy[3]);    
+      step=MIN((unhap/2),pcity->ppl_happy[3]);    
       pcity->ppl_happy[3]-=step;
       pcity->ppl_unhappy[3]+=step;
       unhap -= step * 2;                                                
@@ -293,10 +296,10 @@
 void unhappy_city_check(struct city *pcity)
 {
   if (city_unhappy(pcity)) {
-    pcity->food_surplus=min(0, pcity->food_surplus);
+    pcity->food_surplus=MIN(0, pcity->food_surplus);
     pcity->tax_total=0;
     pcity->science_total=0;
-    pcity->shield_surplus=min(0, pcity->shield_surplus);
+    pcity->shield_surplus=MIN(0, pcity->shield_surplus);
   }  
 }
 
@@ -325,7 +328,7 @@
     pcity->pollution+=poppul;
   }
 
-  pcity->pollution=max(0, pcity->pollution-20);  
+  pcity->pollution=MAX(0, pcity->pollution-20);  
 }
 
 /**************************************************************************
@@ -506,19 +509,19 @@
   switch (gov) {
   case G_ANARCHY:
   case G_DESPOTISM:
-    city_units = min(city_units, pcity->ppl_unhappy[3]);
+    city_units = MIN(city_units, pcity->ppl_unhappy[3]);
     pcity->ppl_unhappy[3]-= city_units;
     pcity->ppl_content[3]+= city_units;
     break;
   case G_MONARCHY:
-    city_units = min(3, city_units);
-    city_units = min(pcity->ppl_unhappy[3], city_units);
+    city_units = MIN(3, city_units);
+    city_units = MIN(pcity->ppl_unhappy[3], city_units);
     pcity->ppl_unhappy[3]-= city_units;
     pcity->ppl_content[3]+= city_units;
     break;
   case G_COMMUNISM:
-    city_units = min(3, city_units);
-    city_units = min(pcity->ppl_unhappy[3], city_units*2);
+    city_units = MIN(3, city_units);
+    city_units = MIN(pcity->ppl_unhappy[3], city_units*2);
     pcity->ppl_unhappy[3]-= city_units;
     pcity->ppl_content[3]+= city_units;
     break;
@@ -1217,14 +1220,16 @@
   else {
     pcity->incite_revolt_cost=city_owner(pcity)->economic.gold +1000;
     capital=find_palace(city_owner(pcity));
-    if (capital)
-      dist=min(32, map_distance(capital->x, capital->y, pcity->x, pcity->y)); 
+    if (capital) {
+      int tmp = map_distance(capital->x, capital->y, pcity->x, pcity->y);
+      dist=MIN(32, tmp);
+    }
     else 
       dist=32;
     if (city_got_building(pcity, B_COURTHOUSE)) 
       dist/=2;
     if (get_government(city_owner(pcity)->player_no)==G_COMMUNISM)
-      dist = min(10, dist);
+      dist = MIN(10, dist);
     pcity->incite_revolt_cost/=(dist + 3);
     pcity->incite_revolt_cost*=pcity->size;
     if (city_unhappy(pcity))
diff -u --recursive --new-file --exclude-from=.diffignore 
freeciv-cvs/server/unittools.c freeciv-minmax/server/unittools.c
--- freeciv-cvs/server/unittools.c      Sat Jun 12 15:03:33 1999
+++ freeciv-minmax/server/unittools.c   Sun Jun 27 22:48:02 1999
@@ -227,12 +227,14 @@
 
   cost = (&game.players[punit->owner])->economic.gold+750;
   capital=find_palace(&game.players[punit->owner]);
-  if (capital)
-    dist=min(32, map_distance(capital->x, capital->y, punit->x, punit->y)); 
+  if (capital) {
+    int tmp = map_distance(capital->x, capital->y, punit->x, punit->y);
+    dist=MIN(32, tmp);
+  }
   else
     dist=32;
     if (get_government(punit->owner)==G_COMMUNISM)
-      dist = min(10, dist);
+      dist = MIN(10, dist);
   cost=(cost/(dist+2))*(get_unit_type(punit->type)->build_cost/10);
   if (unit_flag(punit->type, F_SETTLERS)) 
     cost/=2;

-- 
//Markus