Use feels_like_helping() in dosupport(), lnd_support()
feels_like_helping() case cn == foe is missing in the code it
replaces. No difference in behavior, because:
* cn == foe && cn == friend can't happen. Because you can't get into
ground combat against yourself (assault, attack and paradrop don't
let you), friend != foe for support.
* cn == foe && cn != friend behaves the same: no support.
feels_like_helping() returns 0 because of the explicit case. The
replaced code doesn't support because cn can't be at war with
itself.
Mission execution first builds lists of eligible units, one list per
country. These lists are passed to perform_mission() one by one,
where they get freed.
Bugs:
* unit_interdict() didn't pass the list for the submarine's owner, but
build_mission_list_type() built one. Any submarine movement within
own submarine interdiction mission op areas leaked memory.
* dosupport() passed only lists for countries that actually support
(ally at war with victim), but build_mission_list_type() built lists
for all countries hostile to the victim. Ground combat within
support mission op areas countries that are hostile to one of the
party without actually supporting the other leaked memory.
* perform_mission() failed to free missiles targeting units.
Fixing the latter is straightforward.
Fix the first two by deciding whether a country acts on a mission
trigger before building any lists, in ground_interdict(),
unit_interdict(), dosupport(). Remove the code dealing with that from
build_mission_list_type() and the loops around perform_mission().
lnd_mar() and lnd_mar_one_sector() take an actor argument.
Nevertheless, they sometimes used player->cnum. Fortunately, they are
the same: all callers pass current player for actor. Normalize to
actor for consistency.
Fix land unit attack mobility cost out of allied sectors
Land units pay a mobility penalty when marching into a non-old-owned
sector without sector mobility, to slow them down in newly taken
sectors. Attacking land units pay this penalty regardless of sector
mobility.
When attacking out of an allied sector, the penalty was computed as if
the land unit was owned by that ally. Attacking sectors old-owned by
that ally was too cheap, and taking back one's own was too expensive.
Broken since attacking land units pay the "newly taken" mobility
penalty: commit 2e693275, v4.3.6.
Fix attack when attacking sector gets taken by ally
When an attacking sector got lost while the player was at a prompt,
and the new owner was allied to the player, the server got confused:
1. If the sector attacked with mil, the server let the ghost mil
attack, but not occupy.
2. If the sector was allied, the server reported the sector loss and
land units dropping out of the attack, but claimed the lost sector was
yours.
Fix 1. by dropping sectors from attack when they change owner away
from the player, regardless of relations. Side effect: also drops any
surviving land units there. Before, they dropped out only if the new
owner wasn't allied to the player. That change's okay.
* The check whether the attacker old-owns the attacked sector is
broken, because att_abort() uses sect.sct_oldown uninitialized.
Spotted by the Clang Static Analyzer.
* Its implementation in setrel() is somewhat scary. It's actually
okay, because that part of setrel() only runs within decl(). Other
callers don't reach it: update_main() because player->god != 0
there, and the rest because they never pass a rel < HOSTILE.
* Documentation is a bit vague.
SLOW_WAR hasn't been used in a public game in years. Fixing it is not
worth it, so remove it instead.
The previous commit's message claims the race can lead to duplicated
output, use after free, then double-free. That's correct only up to
the use after free. There is no double-free.
Heap corruption (double-free?) has been observed in Changeling,
though. Player logged in (still in sanctuary), map #, crashed within
removecc()'s free(io->data). Partial backtrace:
raise () from /lib64/libc.so.6
abort () from /lib64/libc.so.6
__libc_message () from /lib64/libc.so.6
malloc_printerr () from /lib64/libc.so.6
removecc (ioq=0x251fd10, cc=468) at ../src/lib/gen/ioqueue.c:350
ioq_dequeue (ioq=0x251fd10, cc=468) at ../src/lib/gen/ioqueue.c:135
io_output (iop=0x251fc90, wait=1) at ../src/lib/empthread/io.c:231
recvclient (cmd=0x258d8e0 "", size=1024) at ../src/lib/player/recvclient.c:82
getcommand (combufp=0x2557068 "map #1") at ../src/lib/player/empdis.c:84
I haven't been able to reproduce.
To hopefully catch ioqueue going south earlier, make ioq_dequeue()
oops when it can't dequeue as many bytes as requested.
Fix race in io_output() that can lead to double-free
Move call of ioq_makeiov() to its use, because calling it before
empth_select() is racy, as follows.
Player thread flushes output by calling io_output(player->iop, 1).
io_output() sets up iov[] to point to queued output. empth_select()
blocks on output.
Another thread sends a C_FLASH or C_INFORM message to this player.
This calls io_output(p->iop, 0). The output file descriptor has
become writable since the player thread blocked on it, so some output
gets written and dequeued.
The player thread resumes, writes out iov[] and dequeues. Any output
already written by the other thread gets duplicated. If the other
thread's dequeue operation freed struct io buffers, there's use after
free followed by double-free.
Don't write garbage to unused trade destination in trade file
struct trdstr members trd_x, trd_y are used only for teleporting
trades. For others, trad() wrote garbage coordinates to the trade
file. They weren't used except by xdump. Fortunately, even there
they're visible only to deities.
Write invalid coordinates instead. Do that in set() as well, so that
coordinates are valid only when we have a teleport destination.
Clean up suspicious coordinate system use in unit_put()
It showed unit coordinates in unit's coordinate system instead of the
actor's. Fortunately, they're the same, since it is reachable only
for non-zero actor, only shp_nav_one_sector(), lnd_mar_one_sector()
and sail_nav_fleet() pass that, and even deities can't navigate
foreign ships or march foreign land units.
Don't limit radar command's range to fit into world map
Limited since Chainsaw 2 so that the radar map fits into a world map
without clipping, i.e. its diameter neither exceeds WORLD_X / 2 nor
WORLD_Y. Maybe range exceeding that triggered bugs then. It doesn't
now, and it makes no sense.
The limit never applied to automatic bmap update from ship radar.
radmap() is now radmap2()'s only caller. Inline radmap2() and
simplify. This cleans up a suspicious-looking use of xyas(): it
relied on the fact that owner == player->cnum if pr_flag.
shp_nav() and shp_nav_one_sector() printed both to their actor
argument and to ship owner. shp_nav_one_sector()'s use of xyas()
looked particularly suspicious: it passed actor, then printed the
result to the ship owner. Fortunately, actor and ship owner are the
same, since even deities can't navigate foreign ships. Normalize to
actor for consistency.
lnd_mar(), lnd_sweep() and lnd_mar_one_sector() printed to the current
player, their actor argument, and to land unit owner.
lnd_mar_one_sector()'s use of xyas() looked particularly suspicious:
it passed actor, then printed the result to the current player or land
unit owner. Fortunately, all three are the same: all callers pass
current player for actor, and land unit owner is the same, since even
deities can't march foreign land units. Normalize to actor for
consistency.
Clean up confusing use of def->own in move_in_land()
It passed def->own to lnd_sweep(), which looks like a bug. But it's
actually player->cnum there, because take_def() already set def->own
to player->owner: take_def() first changes the owner of the attacked
sector by calling takeover(), then updates def->own from that in
att_get_combat().
take_def() and ask_move_in() printed both to the current player and to
land unit owner. Their use of prcom() and xyas() looked particularly
suspicious: they used the current player, then printed the result to
the land unit owner. Fortunately, current player and land unit owner
are the same, since even even deities can't attack with foreign land
units. Normalize to current player for consistency.
Switch get_ototal(), get_oland(), kill_land() and move_in_land() to
current player as well.
The difference between the two is that PR() buffers partial lines, and
mpr() suppresses output to country#0. Doesn't matter when printing
complete lines to a country other than #0, e.g. the owner of a unit.
Since Empire 3 made option NEWPAF mandatory, checkabort is always
non-zero, and show implies checkabort != 1 and other == 0. Drop
unreachable code, and remove unused parameters checkabort and other.
Pass the more obviously correct plane_owner instead of player->cnum.
They're the same, actually.
When deities could still fly foreign planes (before commit 2023b47c),
they weren't. ac_encounter() updated the plane owner's in-memory
bmap, but wrote the current player's bmap to disk.
Fix recon and satellite to write spy plane bmap updates to disk
satdisp_sect() updated the in-memory bmap, but failed to write the
updates to disk. Its callers already update bmap from other sources,
so move this update there, and connect it to the existing write back.
Restrict ac_encounter() mission_flags to current player
The only user is reco(), so the restriction is fine. Several
functions called on behalf of mission_flags assumed it already:
plane_sweep(), sathead(), satdisp_sect(), satdisp_units(). Simplify
the rest accordingly: plane_sona() and ac_encounter() itself.
Fix bogus message when deity attempts to march foreign land unit
Much of the code assumes that only the land unit's owner can march it.
The assumption is correct, because lnd_mar() leaves foreign land units
behind with a bogus "was disbanded at" message (suppressed for country
It would be nice to let deities march foreign land units, but the
assumption is not trivial to remove. For now, just avoid the bogus
message.
Historical note: it looks like deities used to be able to march
foreign land units just fine until Empire 2 factored common code out
of navigate, sail and autonav, and updated march to match navigate.
Likewise, it looks like they could board with foreign land units until
Empire 2 factored out common ground combat code. Commands attack and
assault have always rejected foreign land units, even for deities.
Fix bogus message when deity attempts to navigate foreign ship
Much of the code assumes that only the ship's owner can navigate it.
The assumption is correct, because shp_nav() leaves foreign ships
behind with a bogus "was sunk at" message (suppressed for country #0).
It would be nice to let deities navigate foreign ships, but the
assumption is not trivial to remove. For now, just avoid the bogus
message.
Historical note: it looks like deities used to be able to navigate
foreign ships just fine until Empire 2 factored common code out of
navigate, sail and autonav.
Much code assumes that only the plane's owner can fly it.
pln_airbase_ok() oopses since commit 446f1991. Before, flying planes
from carriers failed with a bogus "not valid for" message, and flying
from sectors had output misdirected to the plane's owner.
It would be nice to let deities fly foreign planes, but the assumption
is not trivial to remove, so just satisfy it for now.
Historical note: it looks like deities used to be able to fly foreign
planes just fine until Chainsaw 3 added missions. The launch command
has always rejected foreign planes, even for deities.
Oops when mpr() is misused for printing partial lines
Such misuse creates a bulletin with a partial line. The read command
normally merges it with the next one, but if the bulletins are more
than five seconds apart (clock jumped somehow), we get a bulletin
header in the middle of a line.
Don't use multiple calls of mpr() to print a single line, because that
creates a separate bulletin for each part. The read command normally
merges the bulletins, but if the bulletins are more than five seconds
apart (clock jumped somehow), we get a bulletin header in the middle
of a line.
While there, wrap long "blam" lines. Can only happen for bomb loads
above 16. Stock game needs a tech 406 jhb for that.
Don't use multiple calls of mpr() to print a single line, because that
creates a separate bulletin for each part. The read command normally
merges the bulletins, but if the bulletins are more than five seconds
apart (clock jumped somehow), we get a bulletin header in the middle
of a line.
Don't use multiple calls of mpr() to print a single line, because that
creates a separate bulletin for each part. The read command normally
merges the bulletins, but if the bulletins are more than five seconds
apart (clock jumped somehow), we get a bulletin header in the middle
of a line.
Don't beep when plane, land unit or nuke die on collapsing bridge
Not nice, because it could beep many times, and could put beeps in
bulletins. Moreover, it misused mpr() and thus put the beep in its
own bulletin. The read command normally merges this bulletin with the
adjacent ones, but if the bulletins are more than five seconds apart
(clock jumped somehow), we can get an empty bulletin just for the
beep.
Output went to the owner of the nuke instead of the player.
Fortunately, they're the same in normal usage. They can differ only
when a deity drops a foreign nuke from a foreign plane.
The fix also cleans up a misuse of mpr() in kaboom(): it used multiple
calls to print a single line, which creates a separate bulletin for
each part. The read command normally merges the bulletins, but if the
bulletins are more than five seconds apart (clock jumped somehow), we
get a bulletin header in the middle of a line. Fortunately, that
could happen only when a deity drops a foreign nuke. Before commit a269cdd7 (v4.3.23), it could also happen for missions.
Clean up suspicious coordinate system use in unit_list()
It showed unit coordinates in unit's coordinate system instead of the
player's. Fortunately, they're the same, since even deities can't
navigate foreign ships or march foreign land units.
Fix misdirected prints when missile explodes on launch
msl_launch() printed some lines to the player instead of the missile
owner when the missile exploded on launch. They are different when
the launch is for a mission or an interception. This disclosed the
the owner's origin. Broken in Empire 2.
Clean up suspicious coordinate system use in lnd_mar()
When refusing to march foreign land units, it reported the land unit's
location in the land unit's coordinate system instead of the player's.
Fortunately, they're the same, since even deities can't march foreign
land unit.
When autonav reported to a ship owner that it can't load or unload
foreign civilians, it used the sector owner's coordinate system. This
disclosed the sector owner's origin. Abusable.
Fix nuclear detontation to use correct coordinate system
When the nuke bounced off a sanctuary, the bulletin to the sanctuary
owner used the attacker's coordinate system. This disclosed the
attacker's origin.
Fix sabotage command to use correct coordinate system
It reported the spy's location in the spy's coordinate system
instead of the player's. Fortunately, they're the same in normal
usage. They can differ only when a deity uses a foreign spy.
It reported the ship's location in the ship's coordinate system
instead of the player's. Fortunately, they're the same in normal
usage. They can differ only when a deity uses a foreign ship.
It reported the engineer's location in the engineer's coordinate
system instead of the player's. Fortunately, they're the same in
normal usage. They can differ only when a deity uses a foreign
engineer.
The buggy code is also reachable from and march sub-command 'd', but
can't bite there, because even deities can't march foreign land units.
Fix nation command to use correct coordinate system
It reported capital location in the nation's coordinate system instead
of the player's. Fortunately, they're the same in normal usage. They
can differ only when a deity requests a nation report for another
country.
Fix sweep command to use correct coordinate system
When reporting sweeps, it reported the location in the plane owner's
coordinate system instead of the player's. Fortunately, they're the
same in normal usage. They can differ only when a deity flies foreign
planes.
projects / empserver / log