When d of n cargo items are discarded for want of space, pln_dropoff()
reported -d items discarded and -d items unloaded. Already broken in
BSD Empire 1.1.
Old version recognizes the first ':', which prevents use of ':' in
host names. They are used in numerical IPv6 addresses. New version
recognizes the last ':', which prevents use of ':' in service names.
Old version treats empty host or port specially (use default).
Documentation suggests ':' is required, but the code doesn't do that.
Instead, the argument is interpreted as host, even when it's empty.
New version makes the HOST: part optional. You can't specify host and
default the port. Tough. Keeps documentation and code as simple as
possible.
Compare:
old version new version
argument host port host port
"" "" default default ""
"A" "A" default default "A"
":" default default "" ""
"A:" "A" default "A" ""
":B" default "B" "" "B"
"A:B" "A" "B" "A" "B"
Both coas() and skyw() want to iterate over a circular area with
radius vrange. They did that by iterating over a rectangle that
encloses the circle, skipping coordinates out of range. To "save
time", they used a rather odd predicate for "out of range", namely
"vrange * vrange < (j * j + 3 * k * k) / 4)".
The predicate is wrong. coastwatch and skywatch could see one sector
too far in certain directions for practical radar ranges, and up to
two sectors for not so practical tech 1900+ radar stations.
For instance, with j = 13 and k = 3, vrange = 7, the predicate
evaluates to false (49 < 48), i.e. in range. However, the true
distance is 8, i.e. out of range. Likewise, j = 22, k = 8, vrange =
13: 169 < 169, true distance 15.
Fix by iterating over the circle directly, without comparing
distances.
Before commit a269cdd7, pln_damage() returned zero when the damage was
nuclear, and callers used that to bypass conventional damage code.
Zero value can't happen anymore.
The server aborts the current command when it receives a special line
of input for a prompt. To make the client send it, you type the INTR
character (normally ^C). This sends the client the SIGINT signal.
Unfortunately, it never quite worked.
Because we use a special line of input to signal interrupt, the client
can do that only after a complete line of input.
What if SIGINT arrives in the middle of a line? We split the line in
two then and there, by inserting a newline. Nasty, but it's simple,
and happens rarely.
However, we inserted the newline always, even after a complete line.
In that case, we inserted an empty line of input before the interrupt.
If you hit INTR at a server prompt, the server received an empty line
of input for that prompt, and the interrupt only for the *next*
prompt. Which may well be too late to abort the command you wanted to
abort.
Fix by inserting the newline only when needed.
Ships can expend shells to defend against missiles, in
shp_missile_defense(). Any shell use by the target ship got wiped out
when shp_missile_interdiction() wrote back the target ship, triggering
a seqno mismatch oops.
Ships get updated when they launch planes to intercept interdicting
planes, in mission_pln_equip(). Any petrol use by the target ship got
wiped out when shp_mission_interdiction() wrote back the target ship,
triggering a seqno mismatch oops.
Fix by re-reading the target ship in shp_damage_one(). This also
affects shp_fort_interdiction(), where it is not necessary. A bit
inefficient, but let's keep this fix simple.
do_unit_move() reads the ships into a list. It re-reads them when it
prompts for sub-commands. shp_nav_one_sector() writes them back when
it moves ships.
Mine-laying (sub-command 'd') updates the minelayer, invalidating the
copy in the list. Any movement sub-command before the next prompt for
sub-commands wiped out this update, triggering a seno mismatch oops.
Happens only if 'd' is used without arguments, because remaining
sub-commands are discarded when there are arguments.
Broken when mine-laying was added in commits 2438fe7c, v4.3.7.
Same for march, commit 274c8e42, v4.3.7.
Fix by stopping after 'd' regardless of arguments.
When sub-command 'd' was used without arguments, do_unit_move() failed
to supply the second argument to mine(), which duly prompted for it.
This contracticted info, and could trigger a generation oops.
do_unit_move() reads the ships into a list. It re-reads them when it
prompts for sub-commands. shp_nav_one_sector() writes them back when
it moves ships.
The mine prompt made the list stale. Movement sub-commands before the
next prompt for sub-commands wrote back stale ships, triggering a
generation oops. Example: "nav 15 dg".
Broken when mine-laying was added in commits 2438fe7c, v4.3.7.
Same for march, commit 274c8e42, v4.3.7.
Commit a269cdd7 (v4.3.23) removed the nuclear damage. But it left the
nuke on the missile, which made pln_damage() oops and return zero
damage.
Fix by destroying the nuke separately.
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.
