Fix prewrite callbacks' cargo list update for in-place update
authorMarkus Armbruster <armbru@pond.sub.org>
Sun, 10 Jul 2011 13:36:24 +0000 (15:36 +0200)
committerMarkus Armbruster <armbru@pond.sub.org>
Sun, 10 Jul 2011 19:10:56 +0000 (21:10 +0200)
When updating in-place (old==new), we must not write through new
before we're done reading the same memory through old.

Bug: we write the carrier uids too early.  Cargo lists aren't updated
when a carrier dies in an in-place update.  No such updates are known.
Broken in commit 64a53c90, v4.3.17.

src/lib/subs/land.c
src/lib/subs/nuke.c
src/lib/subs/plane.c

index 5a7ae3e63655179b485d45b79385494f0ba1d694..14188496a4fa0192979c7ce76fa2e5bbfa5dd13d 100644 (file)
@@ -64,21 +64,24 @@ lnd_prewrite(int n, void *old, void *new)
     struct lndstr *oldlp = old;
     struct lndstr *lp = new;
     natid own = lp->lnd_effic < LAND_MINEFF ? 0 : lp->lnd_own;
+    int ship = lp->lnd_ship;
+    int land = lp->lnd_land;
+
+    /* Be careful with writing to *lp, in case oldlp == lp */
 
     if (!own) {
        lp->lnd_effic = 0;
-       lp->lnd_ship = lp->lnd_land = -1;
+       ship = land = -1;
     }
     item_prewrite(lp->lnd_item);
 
-    if (CANT_HAPPEN(lp->lnd_ship >= 0 && lp->lnd_land >= 0))
-       lp->lnd_land = -1;
-    if (oldlp->lnd_ship != lp->lnd_ship)
-       lnd_carrier_change(lp, EF_SHIP, oldlp->lnd_ship, lp->lnd_ship);
-    if (oldlp->lnd_land != lp->lnd_land)
-       lnd_carrier_change(lp, EF_LAND, oldlp->lnd_land, lp->lnd_land);
+    if (CANT_HAPPEN(ship >= 0 && land >= 0))
+       land = -1;
+    if (oldlp->lnd_ship != ship)
+       lnd_carrier_change(lp, EF_SHIP, oldlp->lnd_ship, ship);
+    if (oldlp->lnd_land != land)
+       lnd_carrier_change(lp, EF_LAND, oldlp->lnd_land, land);
 
-    /* We've avoided assigning to lp->lnd_own, in case oldlp == lp */
     if (oldlp->lnd_own != own) {
        lost_and_found(EF_LAND, oldlp->lnd_own, own,
                       lp->lnd_uid, lp->lnd_x, lp->lnd_y);
@@ -87,6 +90,8 @@ lnd_prewrite(int n, void *old, void *new)
     }
 
     lp->lnd_own = own;
+    lp->lnd_ship = ship;
+    lp->lnd_land = land;
     if (!own || lp->lnd_x != oldlp->lnd_x || lp->lnd_y != oldlp->lnd_y)
        unit_update_cargo((struct empobj *)lp);
 }
index 10b7d23070b774f69157c71b6b56c46b4f36ab13..ed342380686d36a5ac0b4f4dea3d308f743fd5f1 100644 (file)
@@ -29,7 +29,7 @@
  *  Known contributors to this file:
  *     Dave Pare, 1989
  *     Steve McClure, 1996
- *     Markus Armbruster, 2006-2008
+ *     Markus Armbruster, 2006-2011
  */
 
 #include <config.h>
@@ -60,21 +60,24 @@ nuk_prewrite(int n, void *old, void *new)
     struct nukstr *oldnp = old;
     struct nukstr *np = new;
     natid own = np->nuk_effic == 0 ? 0 : np->nuk_own;
+    int plane = np->nuk_plane;
+
+    /* Be careful with writing to *np, in case oldnp == np */
 
     if (!own) {
        np->nuk_effic = 0;
-       np->nuk_plane = -1;
+       plane = -1;
     }
 
-    if (oldnp->nuk_plane != np->nuk_plane)
-       nuk_carrier_change(np, EF_PLANE, oldnp->nuk_plane, np->nuk_plane);
+    if (oldnp->nuk_plane != plane)
+       nuk_carrier_change(np, EF_PLANE, oldnp->nuk_plane, plane);
 
-    /* We've avoided assigning to np->nuk_own, in case oldnp == np */
     if (oldnp->nuk_own != own)
        lost_and_found(EF_NUKE, oldnp->nuk_own, own,
                       np->nuk_uid, np->nuk_x, np->nuk_y);
 
     np->nuk_own = own;
+    np->nuk_plane = plane;
 }
 
 char *
index 7b1c89ae8673f5b1a8e33c84f3e2ad9123175508..840c8ecf858eb9a65ef309e9c4363db289dc18e1 100644 (file)
@@ -64,20 +64,23 @@ pln_prewrite(int n, void *old, void *new)
     struct plnstr *oldpp = old;
     struct plnstr *pp = new;
     natid own = pp->pln_effic < PLANE_MINEFF ? 0 : pp->pln_own;
+    int ship = pp->pln_ship;
+    int land = pp->pln_land;
+
+    /* Be careful with writing to *pp, in case oldpp == pp */
 
     if (!own) {
        pp->pln_effic = 0;
-       pp->pln_ship = pp->pln_land = -1;
+       ship = land = -1;
     }
 
-    if (CANT_HAPPEN(pp->pln_ship >= 0 && pp->pln_land >= 0))
-       pp->pln_land = -1;
-    if (oldpp->pln_ship != pp->pln_ship)
-       pln_carrier_change(pp, EF_SHIP, oldpp->pln_ship, pp->pln_ship);
-    if (oldpp->pln_land != pp->pln_land)
-       pln_carrier_change(pp, EF_LAND, oldpp->pln_land, pp->pln_land);
+    if (CANT_HAPPEN(ship >= 0 && land >= 0))
+       land = -1;
+    if (oldpp->pln_ship != ship)
+       pln_carrier_change(pp, EF_SHIP, oldpp->pln_ship, ship);
+    if (oldpp->pln_land != land)
+       pln_carrier_change(pp, EF_LAND, oldpp->pln_land, land);
 
-    /* We've avoided assigning to pp->pln_own, in case oldpp == pp */
     if (oldpp->pln_own != own) {
        lost_and_found(EF_PLANE, oldpp->pln_own, own,
                       pp->pln_uid, pp->pln_x, pp->pln_y);
@@ -86,6 +89,8 @@ pln_prewrite(int n, void *old, void *new)
     }
 
     pp->pln_own = own;
+    pp->pln_ship = ship;
+    pp->pln_land = land;
     if (!own || pp->pln_x != oldpp->pln_x || pp->pln_y != oldpp->pln_y)
        unit_update_cargo((struct empobj *)pp);
 }