When a torpedo attack triggered a return torpedo, the news reported it
to be fired by the attacker instead of the defender.
(cherry picked from commit bb5abd95e0)
Maps are generally drawn into static scratch buffers. Each command
has its own buffers.
Static scratch buffers are safe as long as they're never used across
yields. Player output can yield unless the command has flag C_MOD
set. Commands lradar, path, radar, route, satellite, sect, survey
hadn't. If such a command yields while using scratch buffers, another
instance of the command can clobber them.
Abuse seems tricky, but possible: if a malicious player stalls output
just right, a command yields while printing a map from the scratch
buffer. It resumes only when the malicious player reads some output.
If another player runs the same command before that, it overwrites the
same static scratch buffer with its map. The malicious player
receives the last such run's map.
4.2.8 fixed the same bug for bmap, lbmap, lmap, map, nmap, pbmap,
pmap, sbmap and smap.
All were broken in 4.2.0. Except radar maps (lradar and radar) were
already broken in Empire 2 for AIX.
(cherry picked from commit 8bdb5c5c1b)
Island size is randomly chosen from the interval [1..2*is+1], with
expected value is. Use two dice to roll the size instead of one.
This makes extreme sizes much less likely.
Output journaling was cherry-picked from Hvy Metal II. However, how
threads are identified in the journal changed since then.
journal_output_1() needs updating for that.
Output often arrives in chunks other than lines. Hard to read in the
journal. Delay journalling until we got a full line or our buffer is
exhausted. This is less precise, but it'll do for now.
To enable, set econfig key keep_journal to at least 2. Output events
are *not* flushed to disk immediately.
Put it in Hvy Metal II now to gather real data for future testing of a
journal replay tool.
Redundant information, but makes the journal easier to read. The
redundancy might help making a journal replay tool robust.
Put it in Hvy Metal II now to gather some real data.
orde() uses a "clever" trick to get away with just one
check_ship_ok(): it makes a copy of the ship in the beginning, and
checks it at the end. Generation numbers (commit 2fa5f652) break this
trick. It could be fixed by manually marking the ship fresh.
Instead, check the ship the stupid, straightforward way, without a
copy.
The call to WSAEnumNetworkEvents() was reseting network
events in particular WSAECONNRESET was sometimes being lost
causing the client to not exit. The WSAEnumNetworkEvents()
was called from fd_is_socket() and has been replace with
getsockopt() to determine if the fd is socket or not.
Broken in commit 4d40a275, v4.3.23
For an argument consisting of a valid path plus whitespace,
do_unit_move() eventually passed the whitespace suffix to parse(),
then dereferenced player->argp[0]. But that was null.
Broken in commit 0c12d837, v4.3.7. Trivial for players to trigger.
When the input line contains only whitespace, player->argp[0] is null
after parse(). This condition was not checked, comtch() returned
M_IGNORE, and code handling that printed the null pointer. Some
systems (GNU, Windows) deal gracefully with printing null strings,
others crash. Trivial to trigger remotely.
Before the fix, the value of parse() was assigned, but never used, and
that was spotted by the Clang Static Analyzer.
If there is no runnable thread, we're deadlocked. lwpReschedule() has
an assertion to catch this. But it didn't work, and the code
following it would crash then. Can't actually happen, because the
EventHandler thread is always runnable. Spotted by the Clang Static
Analyzer.
Local analysis can now easily find out what's up. Before, inter-
procedural analysis was required. The Clang Static Analyzer
complained about a dereference of res that is actually fine.
Local analysis can now easily find out what's up. Before,
whole-program analysis was required. The Clang Static Analyzer
complained about code that is actually fine.
Movement stops when shp_interdict() or lnd_interdict() report
interdiction. However, they reported it only when there was
interdiction damage.
Zero interdiction damage commonly happens when interdicting missiles
miss, or all bombers abort. Stopping regardless of damage makes more
sense there.
Moreover, not stopping is buggy: do_unit_move() needs to take care not
to wipe out updates made by interdiction to the moving ships or land
units. It does so only when it stops. Updates made by interdiction
without interdiction damage could get wiped out, triggering a seqno
mismatch oops.
Known ways moving ships and land units can get updated by interdiction
despite there is no interdiction damage:
* Interdicting bombers get intercepted by planes based on a navigating
carrier, carrier gets charged petrol. The bug wipes out the petrol
use.
* Marching land units get interdicted by planes, but all planes miss.
Sufficiently large collateral damage to the sector can still damage
the land units. The bug wipes out the damage to land units.
To make shp_interdict() and lnd_interdict() report interdiction
regardless of damage, change lnd_missile_interdiction(),
lnd_fort_interdiction(), lnd_mission_interdiction(),
shp_missile_interdiction(), shp_fort_interdiction(),
shp_mission_interdiction() to return whether there was interdiction.
Before, they returned whether there was damage.
Change unit_interdict(), perform_mission(), perform_mission_land(),
perform_mission_ship(), perform_mission_msl(), and
perform_mission_bomb() to return -1 for no interdiction, so that
callers can distinguish no interdiction from interdiction with no
damage.
getland() is obviously redundant in get_dlist(), because the land unit
can't have changed since the previous read.
What's up with ask_olist() is less obvious. For each capable land
unit, ask_olist() asks the player whether to attack with it. If it
attacks, it gets copied into olist. Since asking the player yields
the processor, these copies can become stale. However, re-reading
fixes that just for the last one. Moreover, the code copes with stale
copies just fine: ask_olist() is always directly followed by
att_get_offense() or att_move_in_off(), and both replace the
potentially stale copy in olist.
ask_olist() and get_dlist() called get_land() with llp->chrp still
null, which made it initialize parts of llp instead of performing its
usual integrity check. att_reacting_units() had the initialization
inline. Change the other two to match, and simplify get_land(). No
functional change.
