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.
fgets() fails on read error or EOF. When it fails, it doesn't touch
the buffer. Since files didn't check its return value, it read an
uninitialized character from the buffer. If that happened to be 'y'
or 'Y', it happily clobbered the game.
Many commands, compiler invocations in particular, are rather long,
and warnings tend to get lost in the noise. Suppress the command
details. Run make with a V=1 parameter for full output.
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.
shp_missile_interdiction() does nothing when there are no suitable
targets. Its users avoid to call it when there are none, probably to
save a few cycles. Optimize shp_missile_interdiction() for that case,
and simplify the callers.
