Damage due to lack of maintenance is now limited by the unit's minimum
efficiency.
Before, units could die. Unfortunately, the update left any embarked
units on their dead carrier. Should have seen this when I fixed a
related bug in commit c2c0d1ff, v4.3.22. Broken for ships and land
units when Empire 2 added their maintenance cost, and for planes when
commit 2e40a4bb (v4.3.4) replaced nuclear stockpiles by nuke units.
The common root cause of these bugs is the update bypassing pre-write
functions (bug#1010856).
If another unit with the same number got built, it picked up the stuck
cargo, triggering the oops from commit 6fb5caf6, which see.
In "stuck on dead carrier" state, units pretty much behave as if their
carrier was still alive, with additional protection from the fact that
a dead carrier can't be damaged or boarded.
The server detects this state on startup since commit 7da9aab5, and
refuses to start.
Oops when stuck cargo snaps to new ship, plane or land unit
When units somehow get stuck on a dead carrier, a new build reusing
the dead carrier's UID picks up its cargo. The cargo gets teleported
to its new carrier when the carrier moves.
Oops when a ship, plane or land unit is created with cargo. To
recover, destroy the cargo.
Avoid false positive generation oops in navigate and march
Commit e3cf1e32 (v4.3.27) created make_stale_if_command_arg() to
permit catching more potential yields on input. Unfortunately, the
implementation of navigate and march sub-commands 'r', 'l' and 's'
breaks it.
do_unit_move() reads units into a unit list at the beginning and at
each stop. It writes them back when they move or sweep. If a unit
changed in the file in between, the changes would get wiped out.
Therefore, do_unit_move() must not yield between stops.
do_unit_move() parses sub-commands into player->argp[], then supplies
defaults for missing arguments, so that code using them (radar(),
do_look(), sona(), mine(), landmine()) won't prompt for missing
arguments. Unclean and brittle. See also commit 28cc236e and commit 45106ab9.
Unfortunately, make_stale_if_command_arg() doesn't recognize the
difference between these defaulted arguments and parsed arguments, so
it makes objects stale, even though the defaulted arguments can't be
missing. If a move or sweep follows, it triggers a false positive
generation oops.
To fix, test "points into argument buffer" (only true for parsed
arguments) instead of "is in player->argp[]". Requires making the
argument buffer accessible: new struct player member argbuf[]. Use it
for parsing commands, in command(), execute(), do_unit_move(). Don't
use it in emp_config(), player_login(), move_ground(), because these
parse something else.
Fix client redirection and execute for non-ASCII characters
The client rejects unauthorized redirection and execute. Its
authorized check always fails for arguments with non-ASCII characters.
The culprit is ring_search(): It compares plain char to unsigned char,
which breaks when char is signed. Believed to be broken in commit 8b7d0b91, v4.3.11.
Note that non-ASCII characters only work in UTF-8 sessions. In ASCII
sessions, the server replaces them, and the authorized check fails.
Works as designed.
Fix execute not to mangle the argument when it prompts for it
The argument is UTF-8. If it's missing, execute() gets it with
getstring(), which mangles non-ASCII characters. Clients reject
mangled file names for security reasons.
Fix by switching to ugetstring(). Missed in commit 69832255, v4.3.11.
Non-ASCII characters are still mangled in ASCII sessions, but that's
not expected to work.
Fix execute to filter input just like the main command loop
When we added input filtering in v4.2.21, we missed the execute
command. Because of that, funny characters can be treated differently
in batch files.
The main command loop uses getcommand(), which filters input.
execute() uses recvclient() directly, and doesn't filter input. Both
feed the command to parse(), which drops unwanted control and
non-ASCII characters.
Input filtering drops unwanted control characters and, for ASCII
sessions, replaces non-ASCII characters by '?'.
Because of that, execute in ASCII sessions drops non-ASCII command
characters in batch files rather than replacing them.
Except where parts of the command are used raw: execute's command
echo, flash and wall message argument. There, unwanted control
characters aren't dropped in UTF-8 sessions, and non-ASCII characters
are interpreted as UTF-8 in ASCII sessions. Output filtering replaces
any resulting non-ASCII characters.
players column "last command" also uses the command raw, but commands
executed from batch files are not visible there, so it isn't affected.
To get rid of the differences, move the prompting from getcommand() to
command(), then switch execute() over to getcommand().
Side effect: the batch file's commands are now recorded in
player_commands[]. That's desirable.
copy_utf8_to_ascii_no_funny() eats the character following a replaced
non-ASCII character. Buffer overrun possible when the terminating
zero gets eaten. Broken in commit b5ff7e3b, v4.2.21.
Affected commands:
* players column "last command" in ASCII sessions: struct player
member combuf is UTF-8, uprnf() filters to ASCII.
* read in ASCII sessions: telegram chunks are UTF-8, uprnf() filters
to ASCII.
* flash and wall with message argument in ASCII sessions: argument is
used raw, i.e. UTF-8, pr_flash() filters to ASCII. Safe as long as
we have input filtering sanitizing the raw argument. command() does
that, but execute() doesn't (bug, to be fixed in a later commit).
* execute prompting for its argument in UTF-8 sessions: prmptrd()
receives user text, and filters to ASCII.
Unaffected:
* dispatch() argument redir is UTF-8, uprnf() can filter to ASCII.
Safe as long as we have input filtering sanitizing the raw argument.
command() does that. execute() doesn't, but rejects redirections
before calling dispatch().
* getele() buffer is UTF-8, uprnf() can filter to ASCII. Safe,
because its contents comes from uprmptrd(), which filters input.
Fix xundump's "value must match" check for split tables
The check applies to selectors with flag NSC_CONST set. It permits
initializing them in new objects, but prevents changing them in
existing objects. For split tables, initialization worked only in the
first part, because new objects were considered old in later parts.
For instance, in a custom config sect-chr with mnem in the second
part's field 2, new sector types were rejected with `Value for field 2
must be ""'.
"version" is a normal table since commit da8a1dae, v4.3.12. xdump.pl
wasn't updated for that, and queried the version table twice. When
the deprecated special "xdump ver" was removed in commit 78b3af20
(v4.3.27), the extra query broke. Remove it.
Fix empdump not to touch plane file when import fails
pln_zap_transient_flags() fixes up planes stuck in the air (commit 7ca4f412, v4.3.12). Since commit 4e9e58bf (v4.3.14), it writes back
the fixed planes. This is wrong for empdump.
empdump should touch data only on successful import. When it fails
because ef_verify() fails, and any planes are found stuck in the air,
the plane file gets rewritten.
Make parameter ef_verify() take parameter may_put to let empdump
suppress the plane write-back. The plane file still get written out
on successful import, along with the other imported game state.
Verify game state and configuration reference sanity
Table elements reference other table elements. Bad things happen when
references dangle. ef_verify() already checks whether the referenced
table elements exist. This commit makes it check whether the elements
are "in use". This catches stuff like living planes on dead carriers.
verify_row() refrains from rejecting zero uids, because some tables
may contain blank entries, with zero uid.
Change it to check only header sanity for entries that are not in use.
This filters out all legitimately blank entries. Tighten up the uid
check.
For computing "in use", factor empobj_in_use() out of xdvisible().
Note that xdvisible()'s case EF_COUNTRY doesn't bother to check
nat_stat, because that's implied by what it does check. It's not
implied in empobj_in_use(), so add it there.
Trade ships are now enabled when a ship type with capability trade
exists. No such type exists by default; to enable trade ships,
deities have to customize table ship-chr.
Before, trade ship types were ignored when option TRADESHIPS was
disabled. Except for xdump ship-chr, which happily dumped unusable
trade ship types.
Only trade ships can be auto-scuttled. orde() rejects scuttle orders
for other ships. scuttle_it() double-checks, but gets the test wrong:
it rejects only when opt_TRADESHIPS is enabled. Fix that. While
there, make it oops on inadmissible ships.
Fix how configuration tables and empdump treat omitted entries
Change xundump to blank out omitted rows. Before, they were left
alone. Impact:
* No change for reading builtin tables.
* Reading custom tables now replaces the builtin tables instead of
sort-of-merging them. Wasn't a real merge, because it dropped
builtin entries after the last custom entry, except for
non-truncatable tables item, sect-chr and infrastructure.
* empdump -i now replaces the old state instead of sort-of-merging the
dump into the old state. Wasn't a real merge, because it dropped
old state entries after the last entry in the dump, except for the
fixed-size tables sect, nat, realm and game.
Don't misinterpret blank configuration entries as sentinels
Configuration table entries not defined by builtin and custom
configuration files remain blank. They get misinterpreted as sentinel
in tables that use one. Affected are tables product, ship-chr,
plane-chr, land-chr and nuke-chr. Tables item, sect-chr and
infrastructure are immune despite using a sentinel, because omitting
entries is not permitted there.
Code relying on the sentinel fails to pick up entries after the first
blank one. They don't get set up correctly, they're invisible to
build and show, and not recognized as symbolic selector values (the
frg in ship ?type=frg). xdump is fine, because it doesn't rely on
sentinels. It dumps blank entries normally.
The bugs don't bite in the stock game, because the builtin
configuration files are all dense.
The sentinels are all null strings. Set them to "" in the affected
tables' oninit callback. Fix up code iterating over the tables to
ignore such entries. This is precisely the code relying on sentinels,
plus xdump's xdvisible().
Permit omitting rows at the end of tables nat and game
When not enough rows are supplied for a table with fixed size, treat
the rows missing at the end just like rows omitted elsewhere: make
them blank if the omission is permitted (tables nat and game), else
fail (tables sect and realm; no change).
Forbid omitting rows for tables with const fields: item and sect-chr.
This is consistent with the rule for truncation.
The server expects certain entries in these two tables, and
malfunctions when they're blank. Omitting them in the builtin tables
has always left them blank, but deities are not supposed to edit them,
and maintainers are supposed to know what they're doing, so the issue
was deemed unimportant and ignored. However, I plan to blank out
omitted rows in custom tables as well, and then the issue isn't
unimportant anymore.
Truncating is actually fine for table product. It's forbidden because
selector sname has flag NSC_CONST set. I don't remember why sname was
made const in commit 445dfec9 along with item selector mnem, sect-chr
selector mnem and infrastructure selector name. Unlike the other
tables, no code depends on product's builtin values. Clear the flag.
Overhaul how xundump keeps track of current object
Factor tracking of cur_type, cur_obj, cur_id and cur_is_blank into a
set of functions. They replace getobj(). While there, improve some
error messages.
Move pln_oninit(), lnd_oninit(), nuk_oninit() to filetable.c
They set up invariants, and thus should be always active, not just in
the server. Since ef_blank() isn't used for these files outside the
server right now, this isn't a bug fix, just cleanup.
Move file initialization from files.c to empfile oninit()
files.c writes initial contents to game state files with fixed size.
Necessary for setting up invariants, such as struct sctstr members
sct_x, sct_y matching sct_uid.
Do that from the oninit() callback, so ef_blank() sets up invariants
correctly. Since ef_blank() isn't used for these files right now,
this isn't a bug fix, just cleanup.
Instead of stuffing NULL initializers into the cache initializer
macros UNMAPPED_CACHE(), ARRAY_CACHE(), PTR_CACHE(), ARRAY_TABLE().
In preparation of having some non-NULL initializers.
Fix empdump not to grow game state files with fixed size
empdump -i now complains about extra rows instead of silently growing
the file to a size the server will reject. Affects tables sector,
nation, realms, game.
Bonus fix: better error message on I/O error or insufficient memory.
Fix empdump not to truncate game state files with fixed size
empdump -i now complains about missing rows instead of silently
truncating the file to a size the server will reject. Affects tables
sector, nation, realms, game.
New struct empfile member nent replaces ef_open() parameter nelt.
Cleaner, because the expected size is a property of the file, not of
how it's used. Also fixes empdump to check file sizes.
Complication: with EFF_CREATE, ef_open() creates an empty file, to be
extended to the correct size. Callers passed nelt argument -1 along
with EFF_CREATE, to make ef_open() accept the empty file. Can't do
the same for empfile member nent. Instead, make ef_open() not check
the (zero) size then.
ef_verify() assumes views are open. Bug is harmless, because
ef_nelem() returns zero for closed views, and ef_verify() accesses
only immutable parts of struct empfile then.
New struct empfile member base replaces ef_open_view() parameter base.
Cleaner, because the base table is a property of the view, not of how
it's used.
Use it to clean up verify_fail()'s base table access, and for extra
sanity checks in ef_open() and ef_open_view().
Change struct empfile callback onresize() to return void
ef_open() handles onresize() failing incorrectly. Instead of fixing
that, drop the failure mode. It's not really used: unit_onresize()
fails only when used incorrectly. It isn't. If it ever is, ignoring
the failure is safe.
Make ef_close() clear them always, even for views. Harmless, as
ef_open_view() always sets the same flags.
Drop redundant assignment to ep->flags in ef_open(). ef_close()
clears mutable flags, no need to clear them some more right before
calling it. Missed in commit 3eb3607f, v4.3.0.
ef_open() and ef_close() clear EFF_SENTINEL because of that. Broken
since commit 1492845c (v4.3.17) added EFF_SENTINEL. Harmless, as no
file-backed table has a sentinel.
Fix ef_open()'s error message for file larger than static cache
The message claims the file is larger than it actually is. Broken
since commit 71908018 (v4.3.0) implemented static cache in ef_open().
Harmless, because no file-backed table has a statically allocated
cache.
Fix how empdump rejects attempt to split table sect
Split tables require the record index in the leftmost column.
defellipsis() correctly rejects "..." when the table doesn't have one.
It fails to reject it when it has one that is NSC_EXTRA, and thus not
permitted in a dump. This is the case for table sect. defellipsis()
happily succeeds, then chkflds() demands column "uid" if it's missing,
and rejects it if its present.
It missed those with more flags than just NSC_EXTRA set: table sect
name uid, table nat names passwd, xorg, yorg, contacts, rejects.
Since xundump() doesn't provide space for these, the bug could lead to
buffer overruns. Fixes flawed commit 726a8e3d, v4.3.12.
xubody() neglected to check ef_truncate()'s return value. Two failure
modes: invalid arguments, and ftruncate() failure. The former
shouldn't happen, and the latter can happen only for file-backed
tables, hence only in empdump -i.
Tables with a file name are: any game state, and any table that's
initialized from a .config file.
Tables that are no longer customizable: "updates" (customization had
no effect, because update_get_schedule() overwrote it), "table",
"meta" and the symbol tables (customization couldn't change them
anyway), and news-chr (customizing r_newsstory[] was kind of neat, but
unsafe because they are format strings for sprintf()).
"xdump updates" believes there are always 15 (UPDATE_TIME_LEN - 1)
scheduled updates. When fewer than 15 updates are scheduled, it shows
whatever crap update time happens to be in the unused part of
update_time[]: the initial zero or a previously scheduled update.
Root cause is that table EF_UPDATES has always UPDATE_TIME_LEN - 1
entries, which is incorrect when fewer updates are scheduled. Only
xdump is affected, as the other users ignore the length and stop at
the sentinel.
Fix update_get_schedule() to resize table EF_UPDATES.
Provide proper ca_table for carrier unit# selectors
Makes ef_verify() check carrier UIDs are sane. Partially protects
unit_cargo_init(), which oopses on bad carriers.
This has become possible only since commit 64a53c90 (v4.3.17) set
their values to -1 in newly created units. Before, they were zero in
units that had never been used, and a proper ca_table would have made
ef_verify() fail when unit#0 didn't exist.
The only unit# selectors left without a proper ca_table are ship's
follow, lost's id and trade's unitid. Document why.
Update examples in doc/xdump to current server's output
The NSC_CONST flags added in commit fa63f87b have always been missing
here. The previous commit changed xdump meta meta. Other than that,
it's just different encodings.
Refer to table names instead of C identifiers in .config
The C identifier permits looking up the table in the source. The
table name permits lookup with xdump.
Coders should know how to go from table name to C identifier. Deities
aren't all coders; we shouldn't ask them to guess table names from C
identifiers.
Don't ignore non-virtual NSC_EXTRA columns in ef_verify.c
These are commonly timestamps (no verification implemented), or
aliases for a non-extra column (which gets verified). Commit 49780e2c
(v4.3.12) added the exception: EF_SECTOR's uid. Proof by example that
ignoring these columns is wrong. Fix: ignore only virtual columns.
Simplify buil(): replace a switch by a function pointer
To enable that, make build_ship() & friends all take the same int type
argument instead of each one its own pointer. Passing pointers
triggered "may be used uninitialized" compiler warnings (the code was
safe despite the warnings).
Don't truncate research before multiplying with drnuke_const
For drnuke_const 0.33, research level 92.4 now suffices for a tech 280
nuke. Before, you needed 93, which was inconsistent with what
version's promise "need 0.33 times the tech level in research".
This was probably neglected when the techlists feature was added in
v4.0.0, because compiled-in nukes were sorted by tech, unlike ships,
planes and land units. Customization can break that.