From a8b7d1d017da35cc6b1b88c4191b718828e738e8 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sun, 10 Jul 2011 15:36:24 +0200 Subject: [PATCH] Fix prewrite callbacks' cargo list update for in-place update 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 | 21 +++++++++++++-------- src/lib/subs/nuke.c | 13 ++++++++----- src/lib/subs/plane.c | 21 +++++++++++++-------- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/lib/subs/land.c b/src/lib/subs/land.c index 5a7ae3e63..14188496a 100644 --- a/src/lib/subs/land.c +++ b/src/lib/subs/land.c @@ -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); } diff --git a/src/lib/subs/nuke.c b/src/lib/subs/nuke.c index 10b7d2307..ed3423806 100644 --- a/src/lib/subs/nuke.c +++ b/src/lib/subs/nuke.c @@ -29,7 +29,7 @@ * Known contributors to this file: * Dave Pare, 1989 * Steve McClure, 1996 - * Markus Armbruster, 2006-2008 + * Markus Armbruster, 2006-2011 */ #include @@ -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 * diff --git a/src/lib/subs/plane.c b/src/lib/subs/plane.c index 7b1c89ae8..840c8ecf8 100644 --- a/src/lib/subs/plane.c +++ b/src/lib/subs/plane.c @@ -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); } -- 2.43.0