Notable gaps in its coverage are fallout, most of guerrilla, delivery,
distribution, ALL_BLEED and LOSE_CONTACT.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Add edit u keys 'A' for plague stage, and 'b' for plague time.
Admittedly unobvious, but at least they match edit s keys 'a' and 'b'.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Exercise version, show and xdump, except for xdump of game state.
The xdump part is mostly factored out of tests/smoke.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
One-way sorties (fly, recon and sweep) reject mountain destinations
with a "Nowhere to land" message. However, planes can land there just
fine when they return to base (bomb, drop, paradrop, missions).
Already inconsistent in BSD Empire 1.1.
Fix the inconsistency by changing pln_where_to_land() to permit only
helicopters to land in mountains, and pln_airbase_ok() to permit only
helicopters and missiles to take off there, i.e. reject fixed-wing
aircraft.
The flying commands now reject fixed-wing planes based in mountains
with an "is in a mountain and can't take off" message.
Commands flying to a mountain now select only helicopters and silently
ignore the rest, exactly like they select only VTOL planes for flying
to a non-airfield. If no planes can be selected, the command fails
with a "No planes could be equipped" message. This is admittedly less
clear than the "Nowhere to land" message we got before.
Missions now ignore fixed-wing planes based in mountains, exactly like
they ignore non-VTOL planes outside airfields. This may make players
wonder why the fixed-wing VTOL planes they transported up that
mountain don't obey missions. Missions are always quiet unless they
execute.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The two "while it is carrying a nuclear weapon" messages lack
newlines. Add them. Screwed up in commit a269cdd, v4.3.23.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When bombing land units, the bombers get a chance to spot spies. They
can target one even when it wasn't spotted. This makes no sense.
Screwed up when spy units were added in 4.0.0. Hide them completely.
They can still be killed via collateral damage.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
This reverts commit 9b33a4c598.
Parameter only_count was introduced so would_abandon() could use
unitsatxy(), but that was a flawed idea, fixed in the previous commit.
No callers passing non-zero remain, so get rid of it.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
sct_prewrite() makes an owned sector revert to the deity when there
are no civilians, military or own land units.
would_abandon() tries to predict that, but gets it wrong: it ignores
land units that evade spy detection or are loaded on ships, and it
fails to ignore land units loaded on land units marching out.
Broken in commit 7c1b166, v4.3.33. Fix by counting manually rather
than with unitsatxy().
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
quiet_bigdef() runs for each attacker. It lets each eligible defender
fire at most once. The first time a defender is eligible, it fires
and is saved in the list of defenders, along with its firing damage.
If it's eligible again for a later attacker, it's found in the list of
defenders, and the damage is reused. The list of defenders searched
with search_flist(). Unfortunately, search_flist() compares only uid,
not type, and therefore can return a previously found defender of
another type.
If there are multiple attackers and multiple defenders with the same
uid, total damage can be off, damage can be spread to attackers out of
range, and defenders may not be charged shells. Abuse is possible,
but complicated to set up, and probably not worth the trouble.
Broken in commit f89edc7, v4.3.12. Fix by comparing the type as well.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The "loaded on ship" condition was useless from the start (v4.2.0).
The "loaded on land" condition became useless in commit 45d090b,
v4.3.28.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When the player aborts the command at the movement prompt, we write
back stale ships or land units, triggering a generation oops. Any
updates made by other threads meanwhile are wiped out, triggering a
seqno mismatch oops.
Broken in commit 24000b4, v4.3.33. Fix by restoring the lost
shp_nav_stay_behind() and lnd_mar_stay_behind() calls.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When the player declines to abandon a sector, we write back stale land
units, triggering a generation oops. Any updates made by other
threads meanwhile are wiped out, triggering a seqno mismatch oops.
The culprit is lnd_abandon_askyn(): when the player declines, it
returns without calling check_sect_ok(), check_land_ok(). Broken in
commit 7c1b166, v4.3.33. Fix it.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
recvclient() calls ef_make_stale() only when it does actual I/O, via
io_output() and io_input(). Missed in commit 2fa5f652, v4.3.24. Call
it directly when it doesn't do actual I/O.
This makes navi-march-test expose a bug in march: when the player
declines to abandon a sector, we write back stale land units,
triggering a generation oops.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Testing whether the compiler supports it is a bit tricky.
The obvious AX_APPEND_COMPILE_FLAGS([-fstack-protector-strong])
doesn't suffice, since some ports of the GNU toolchain reportedly pass
this test, then fail to link. That's because the compiler accepts the
flag, duly emits references to helper code in libc, but libc doesn't
provide, and linking fails.
Instead, use AX_APPEND_LINK_FLAGS with an input source that makes the
compiler emit the extra stack checking code. This requires the latest
version from the autoconf-archive, so update m4/ax* to commit e3d948b.
Also update m4/my_append_compile_flags.m4 to keep it in sync with
upstream's ax_append_compile_flags.m4.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When savecore can't find a core dump, it reports something like
ls: cannot access core.*: No such file or directory
to stderr, and fails. If privlog is set, it also mails out a "Could
not save core dump" note.
Suppress the error message, and mail out "Could not find core dump to
save" instead.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Contemporary compilers can squeeze out some extra performance by
assuming the program never executes code that has undefined behavior
according to the C standard. Unfortunately, this can break programs.
Pointing out that these programs are non-conforming is as correct as
it is unhelpful, at least as long as the compiler is unable to
diagnose the non-conformingness.
Since keeping our programs working is a lot more important to us than
running them as fast as possible, forbid some assumptions that are
known to break real-world programs:
* Aliasing: perfectly clean programs don't engage in type-punning, and
perfectly conforming programs do it only in full accordance with the
standard's (subtle!) aliasing rules. Neither kind of perfection is
realistic for us, therefore -fno-strict-aliasing.
* Signed integer overflow: perfectly clean programs won't ever do
signed integer arithmetic that overflows. This is an imperfect
program, therefore -fno-strict-overflow.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
MALLOC_CHECK_=3 makes glibc check for memory allocation programming
errors. It's the factory default, but set it anyway just in case
someone disabled it for speed.
Non-zero MALLOC_PERTURB_ makes glibc wipe memory value on allocation
and deallocation. The actual value determines the bit pattern. Set
it to the value of environment variable EMPIRE_CHECK_MALLOC_PERTURB or
else a pseudo-random number, and record it in sandbox/malloc-perturb.
See mallopt(3) for more information.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When the player aborts the command at the movement prompt, or declines
to abandon a sector, unit_move() returns without freeing the list.
Found with valgrind. Broken in commit 24000b4 and commit 7c1b166,
both v4.3.33.
Free the list on these returns, too.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
shp_nav_gauntlet() and lnd_mar_gauntlet() read beyond the list head
when the list is empty. The values read aren't used then. Could
conceivably crash the server anyway, but it's unlikely.
Empty list happens when shp_nav_dir(), lnd_mar_dir() empty the list
and return zero. Broken in commit beedf8d, v4.3.33. Occurs in
navi-march-test (since the last commit) and in retreat-test.
Change shp_nav_dir() and lnd_mar_dir() to return one then. For
additional safety, make shp_nav_gauntlet() and lnd_mar_gauntlet() oops
on empty list and recover safely.
I think I originally found this bug with -fsanitize, but I've since
upgraded, and I can't diagnose it that way anymore.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The code computing the length of the flight path checks whether the
path ends with 'h'. When getpath() returns an empty path, it accesses
flightpath[-1]. This could set the length to -1 (unlikely), or crash
(even less likely). The former could be abused to gain mobility for
sufficiently inefficient or short-ranged planes. Found with valgrind.
Broken in commit 404a76f7, v4.3.27.
Historically, getpath() could return paths with or without 'h', and
the check was necessary. It returned an empty path only when the
player gave no input, aborting the command. When the player entered
the assembly point's coordinates, it returned "h".
Commit 404a76f7 accidentally changed it to return "" then. Also broke
flying to the assembly point's coordinates. Commit 0f1e14f (v4.3.31)
fixed that part by changing getpath()'s contract: always return paths
without 'h' ("" simply means empty path), and return NULL on invalid
input, including no input.
The flawed check is superfluous since then. Drop it.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
There's just one, in show_product().
Use new BUILD_ASSERT() there, because its contract is even simpler
than BUILD_ASSERT_ONE()'s.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
We want to cause a diagnostic when NSC_SITYPE()'s argument isn't
implemented. Commit aa6ad9d's solution is to have the macro expand
into 1/0 then. Works with GCC, but Clang always warns "division by
zero is undefined".
The better, portable way to conditionally break the build is an array
type with a size that's negative when the build should fail, else
positive. Implement that wrapped in a sizeof() to make it an
expression as macro BUILD_ASSERT_ONE(), and use it in NSC_SITYPE().
No more warnings from Clang 3.5.0. GCC still produces its "may be
used uninitialized" false positives.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
We've always squashed them when the time difference is smaller than
TEL_SECONDS, regardless of sign. This involves passing the difference
to abs(), implicitly casting from time_t to int, which triggers a
Clang warning.
I could clean this up to get rid of the warning, but time should never
go backwards, and trying to make things prettier when it does isn't
worthwhile. Simply drop the abs().
While there, drop the function comment. It's been inaccurate since
Empire 3 dropped mail.c, and bogus since commit 17223e8 (v4.3.29)
added tel_cont.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Commit eb1512d (v4.3.6) added the '=' if stopped before efficiency.
Commit 016249c (v4.3.6) changed it to '!' without updating info ship,
plane, land, nuke.
Reported-by: Harald Katzer
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The cost of firing naval guns is 15 mobility with option NOMOBCOST
disabled. Mobility.t is correct.
Fix Options.t not to claim submarines pay half the sector movement
cost when NOMOBCOST is enabled.
Fix fire.t not to claim ships pay half the sector movement cost when
NOMOBCOST is disabled.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Don't list options separately for major server versions. It's only of
historical interest, which "info History" satisfies.
Make it a list (.L) instead of preformatted text (.nf).
Fix up so the option explanations are full sentences, starting with a
capital letter and ending with a period.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
... when referring to a function's parameter or a struct/union's
member.
The idea of using FOO comes from the GNU coding standards:
The comment on a function is much clearer if you use the argument
names to speak about the argument values. The variable name
itself should be lower case, but write it in upper case when you
are speaking about the value rather than the variable itself.
Thus, "the inode number NODE_NUM" rather than "an inode".
Upcasing names is problematic for a case-sensitive language like C,
because it can create ambiguity. Moreover, it's too much shouting for
my taste.
GTK-Doc's convention to prefix the identifier with @ makes references
to variables stand out nicely. The rest of the GTK-Doc conventions
make no sense for us, however.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Renaming carg() would be smarter, but I'd rather do that as part of a
consistent renaming of all command functions, and I'm not up to that
right now.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Using ctime() as pr()'s first argument is safe, because its value
never contains '%'. Clean it up anyway, so we can enable
-Wformat-security.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
