Planes flying one-way with crew or cargo spread plague from their old
base to their new base. Planes dropping cargo spread plague from
their base to the drop's target sector.
msl_equip(), find_escorts() and perform_mission() memset() the plist,
then assign to all members but load. Just zero load instead, like
getilists(), msl_sel() and pln_sel() do.
Flying them to a foreign destination magically changes their
allegiance. Prohibit that.
Equivalent change was already in commit 35887222 (v4.2.17) but got
reverted immediately (commit 20199b22), because fly and drop should
stay consistent with load, which let you give away civilians then. No
more since commit 92a366ce (v4.3.20). This change makes fly and drop
consistent with load again.
New function reads and returns target sector/ship. Avoids reading the
target sector unnecessarily. Callers receive the target ship, not
just its number. Next commit will put it to use.
We test whether the the carrier has space for each plane individually
instead of whether it has space for all of them. The planes that fit
land, the others abort and get teleported home. Abusable.
pln_oneway_to_carrier_ok() was created in commit 1127762c (v4.2.17) to
fix almost the same bug. It worked fine then, because
fit_plane_on_ship() worked with load counters, and incremented them.
Broken in commit 3e370da5 (v4.3.17), which made fit_plane_on_ship()
count the loaded planes, to permit the removal of load counters. But
unlike load counters, loaded planes don't change during
pln_oneway_to_carrier_ok(). Thus, each plane is checked individually.
Fix by tallying all the planes before checking for space.
Tending a negative number of commodities takes from the target ships.
The target ships must be owned. Tend complains when the target
doesn't have the commodity loaded. It does that even for friendly
foreign ships. Don't.
Broken when Chainsaw 2 added tending to allies.
Tending a negative number of commodities takes from the target ships.
When a target ship is foreign, tend silently stops. This is wrong.
Fix it to skip foreign target ships instead.
Broken when Chainsaw 2 added tending to allies.
When a missile explodes on launch, it has a 33% chance to damage its
base.
Unfortunately, damaging the base breaks callers that call msl_launch()
for each member of a list of missiles created by msl_sel() or
perform_mission(). Damage to the base can damage other missiles
there. Any copies of them in the list become stale. When
msl_launch() modifies and writes back such a stale copy, the damage
gets wiped out, triggering a seqno oops.
Affects missile interdiction and interception using missiles with
non-zero load. Stock game's ABMs have zero load, so interception is
safe there. Relatively harmless in practice. Broken in Empire 2.
Instead of fixing the bug, simply disable damage to the base for now.
March code reads the land units into a land unit list, and writes them
back when it changes them, e.g. when a land unit stops. If a land
unit changes in the land unit file while it is in such a land unit
list, the copy in the land unit list becomes stale, and must not be
used.
To that end, do_unit_move() calls lnd_mar() after prompting for path
or destination. lnd_mar() re-reads all the land units.
Unfortunately, it still writes back stale copies in certain
circumstances. Known ways to trigger such writes:
* Deity loads land unit onto a ship or land unit
* Land unit's crew killed just right, e.g. by collateral damage from
interdiction, followed by additional updates, such as shell fire
damage
* Sector no longer owned or allied, e.g. allied sector captured by an
enemy (own sector would kill or retreat the land unit)
Writing a stale copy wipes out the updates that made the copy stale,
and triggers a seqno mismatch oops. For instance, damage that follows
killing of all crew by collateral damage from interdiction is wiped
out. If no damage follows, we still get a generation oops.
Navigation code reads the ships into a ship list, and writes them back
when it changes them, e.g. when a ship stops. If a ship changes in
the ship file while it is in such a ship list, the copy in the ship
list becomes stale, and must not be used.
To that end, do_unit_move() calls shp_nav() after prompting for path
or destination. shp_nav() re-reads all the ships. Unfortunately, it
still writes back stale copies in certain circumstances. Known ways
to trigger such writes:
* Deity sets a sail path
* Ship's crew gone, e.g. killed by shell fire
* Sector no longer navigable, e.g. harbor shelled down, or bridge
built
Writing a stale copy wipes out the updates that made the copy stale,
and triggers a seqno mismatch oops. For instance, ship damage that
kills all crew while the ship is being navigated gets wiped out.
Commit 904822e3 introduced use of SHUT_WR, which Windows calls
SD_SEND. Add the obvious work-around.
Commit 49ae6a7b introduced use of gettimeofday(), which the Microsoft
CRT lacks. Add a replacement based on _ftime_s().
Fairland creates islands with size 1 + random() % (2 * is - 1), where
"is" is either chosen by the user (fourth command line argument) or
defaults to half the continent size (second command line argument).
Negative values are silently replaced by zero.
Not only does value zero make no sense, it also breaks the code: the
island size is always one then (because random() % -1 is zero), but
allocate_memory() provides only space for zero sectors in sectx[],
secty[] and sectc[]. This leads to buffer overruns in try_to_grow(),
find_coast(), elevate_land, set_coastal_flags(). Can smash the heap.
Fix by changing the lower bound from zero to one. Diagnosed with
valgrind. Has always been broken.
elevate_land() tests for capital sector in three places. The third
one is broken: half of the test is done even for islands, subscripting
capx[] and possibly capy[] out of bounds. This could screw up
elevation (unlikely) or crash (even less likely). Diagnosed with
valgrind.
Broken since the test was added in Chainsaw 3.12. Parenthesis were
added blindly 4.0.11 to shut up the compiler. Reindentation (commit
9b7adfbe and ef383c06, v4.2.13) made the bug stand out more, but it
still managed to hide in the general ugliness of fairland's code.
The journal logs a thread name for each event. The player thread name
changes on entry to the playing phase. Connecting old and new name
isn't as easy as it should be:
Sun Apr 29 12:13:39 2012 Conn29 input coun POGO
Sun Apr 29 12:13:39 2012 Conn29 input pass peter
Sun Apr 29 12:13:39 2012 Conn29 input play
Sun Apr 29 12:13:39 2012 Play#0 login 0 127.0.0.1 armbru
Sun Apr 29 12:15:39 2012 Play#0 logout 0
To connect Conn29 with Play#0, you have to know that country#0 is
named POGO.
Fix that by logging login before the thread name change:
Sun Apr 29 12:17:41 2012 Conn29 input coun POGO
Sun Apr 29 12:17:41 2012 Conn29 input pass peter
Sun Apr 29 12:17:41 2012 Conn29 input play
Sun Apr 29 12:17:41 2012 Conn29 login 0 127.0.0.1 armbru
Sun Apr 29 12:19:41 2012 Play#0 logout 0
Now "Conn29 login 0" makes the connection obvious.
This involves moving journal_login() from player_main() before
empth_set_name() in its caller play_cmd(). Move journal_logout() as
well, for symmetry.
If player_main() fails, we now log login/logout instead of nothing in
the journal. That's okay. Note that before commit c9f21c0e (v4.3.8),
we logged just login then.
It happily arms a plane with a remote nuke. The nuke gets teleported
to the plane when the plane moves (a two-way sortie doesn't count as
move). Broken in 4.3.3. Reported by Harald Katzer.
It was renamed to play_lock because it synchronized not just updates
but also shutdown. Since the previous commit, it again only
synchronizes updates. Rename it back.
Also move its initialization next to shutdown_lock's.
shutdwn() sets the EOF indicator, aborts the running command, if any,
forbids sleeping on I/O and wakes up the player thread, for all player
threads in state PS_PLAYING. It takes play_lock to prevent new
commands from running. It then waits up to 3s for player threads to
terminate, by polling player_next(), to let output buffers drain.
Issues:
1. Polling is lame.
2. New player threads can still enter state PS_PLAYING. They'll block
as soon as they try to run a command. Somehwat unclean.
3. We can exit before all player threads left state PS_PLAYING, losing
a treasury update, play time update, and log entries. Could happen
when player threads blocked on output until commit 90b3abc5 fixed
that; its commit message describes the bug's impact in more detail.
Since then, the bug shouldn't bite in practice, because player
threads should leave state PS_PLAYING quickly.
Fix by introducing shutdown_lock: player threads in state PS_PLAYING
hold it shared, shutdwn() takes it exclusive, instead of play_lock.
Takes care of the issues as follows:
3. shutdwn() waits until all player threads left state PS_PLAYING, no
matter how long it takes them.
2. New player threads block before entering state PS_PLAYING.
1. shutdwn() still polls up to 3s for player threads to terminate.
Still lame. Left for another day.
The victim's connection closes without any explanation. Output may be
lost. This is because kill_cmd() kills by calling io_shutdown(),
which shuts down the socket and drains the I/O queues.
How this makes the victim's thread terminate is a bit subtle: shutting
down the socket makes it ready. If the victim's thread is waiting for
I/O, it wakes up. Since all further reads return EOF, and all further
writes fail, the command terminates quickly (short of inifinite loop
bugs), then the command loop, and finally the thread.
To make kill behave more nicely, change kill_cmd() to work exactly
like server shutdown: send a flash message to the victim, set his EOF
indicator, abort the command, forbid sleeping on I/O, wake up the
victim's thread. Just as reliable, but doesn't lose output.
If the victim's client fails to close his connection, the victim's
thread may still linger in state PS_SHUTDOWN for up to
login_grace_time (default 120s). An attacker could try to use that to
make the server run out of file descriptors or memory, but simply
connecting achieves the same effect more cheaply.
