Final change log polish for 4.3.29

Update change log for 4.3.29

drop and fly from carrier can fail to load last civ or mil

pln_equip() refuses to abandon its base sector.  Unfortunately, it
checks even when flying off carriers, and refuses to load the last
civilian or military depending on what happens to be in uninitialized
variable sect.

Broken in commit 91139692, v4.3.0.

Clean up journal_open() to open the journal write-only

We don't actually need update mode.

Clean up read to open telegram file read-only

Update mode hasn't been necessary since 4.0.11 dropped use of
ftruncate().

Check for mailbox creation failure

Deity command add logs the failure, utility program files reports it
and exits unsuccessfully.  Before, this failure was silently ignored.

Factor mailbox_create() out of nat_reset() and files.c

Change read and wire to never delete a corrupt mailbox

Before, rea() deleted the mailbox regardless of errors.  Acceptable
only when the user gets a chance to avoid that after the problem is
reported.  Not the case for "read y".

Not an issue for announcements, but treat them the same for
simplicity.

Clean up how read deals with new telegram arrivals

Fooling around with the file size is silly.  It works only because
read has flag C_MOD set, so they can only arrive while we're sitting
at the delete confirmation prompt, not during reading.

Simply try to read more telegrams instead.

Change wire not to read more after confirming deletion

Telegram deletion deletes the mailbox.  If more telegrams arrive while
we wait for the player to confirm deletion, the mailbox again contains
unread telegrams, so we can't just delete it.  Instead, rea() loops to
read the new telegrams.

Announcements worked the same until Empire 3 put them in a single file
shared by all.  Since then, deleting announcements merely updates
nat_annotim, and there's no need to read new announcements after
getting the player's confirmation.  So don't.

Limit telegram squashing to 5s total

Before, only the time between adjacent telegrams was limited, not the
total time.

Don't lie "You have a new telegram" after read with inform off

Here's how telegram notification works with NF_INFORM off: typed_wu()
increments the telegram recipient's nat_tgms.  status(), running right
before command prompts, notifies the player when nat_tgms > 0, and
resets it.  Thus, we tell the player how many telegrams arrived since
the previous command prompt.

However, what we really want is something else, namely the number of
"new telegrams waiting".  That's what the notification message says,
after all.  Telegrams already printed by read shouldn't count, even
when they arrived since the previous command prompt.

Make them not count by clearing pending telegrams on read regardless
of toggle inform.

Same for announcements.

Fix telegram notifications during read (toggle inform on)

Reset number of pending telegrams before delete prompt instead of
after.

Before, the client claimed pending telegrams at that prompt, because
it wasn't C_INFORMed of the read, yet.  Worse, if more telegrams
arrived while sitting at the prompt, the reset clobbered their number
and sent a bogus clear C_INFORM message, effectively hiding the new
arrivals from the player.

Fix wire not to clear pending telegrams with toggle inform on

Broken since Empire 2 introduced toggle inform.

Fix pending anno count for annos squashed together

Adjacent announcements are squashed together when the sender is the
same and the timestamp is "close enough".  Except typed_wu()
increments natstr member nat_ann regardless.  Fix that to work exactly
like nat_tgms.

Fix read not to split production report when update is slow

Adjacent telegrams are squashed together if type and sender are the
same, and the timestamp is "close enough".  This is done in two
places: rea() and typed_wu().  They're inconsistent: typed_wu()
ignores the timestamp for production reports since Empire 2, but rea()
doesn't.

Record typed_wu()'s decision in new telstr member tel_cont.  Use it in
rea().

Simplify how typed_wu() counts telegrams

typed_wu() counts telegrams to update nat_tgms and, since Empire 2,
send C_INFORM messages.  Adjacent telegrams are squashed together if
type and sender are the same, and the timestamp is within TEL_SECONDS.

typed_wu() increments nat_tgms when it sends a telegram that read
doesn't squash into the previous one.

Since Empire 2, it also sends a C_INFORM message then.  Inexplicably,
it fails to use the same condition: it tests just new_tele, not
new_tele || np->nat_tgms == 0.  C_INFORM messages got missed, until
4.0.18 made rea() call clear_telegram_is_new().  Convoluted.

Send C_INFORM exactly when incrementing nat_tgms, and back out
4.0.18's fix.

Fix clear_telegram_is_new() for TEL_NORM from POGO at the epoch

Purely theoretical, of course.

Clean up how telegram_is_new() detects production report

Test for TEL_UPDATE, not update_running.

Move clear_telegram_is_new() call to beginning of update

The call was added in 4.2.5 "so that the next telegram is flagged as
new and not part of the update".  Since the update sends only
TEL_UPDATE telegrams (the previous commit restored that property), and
nothing else does, the next telegram is flagged as new automatically,
except when it's from the next update.  Document that, and move the
call to a more natural place.

Ensure the update's production report isn't split by bulletins

During the update, wu() sends TEL_UPDATE telegrams ("Production
Report") instead of TEL_BULLETIN telegrams, but typed_wu() has no such
logic.  It's used by tele(), which doesn't run during the update, and
mpr(), which may, e.g. called from ship or mission code used by
autonav or sail.  This inserts bulletins in the middle of the
production report, splitting it apart.

Happens since mpr() was added in Empire 2.  Before, only tele() used
typed_wu() directly, and everything else wu().

Change mpr() to use wu().

Saner error handling in typed_wu()

Leave nat_ann, nat_tgm alone and return -1 on all errors.  Before,
only failed open was handled that way.  Failed write and close were
logged and ignored.  While there, improve the log messages a bit.

Note: the return value fix has little effect.  It makes tele() log the
failure, which is redundant.  Everything else goes through wu() and
ignores the value.

Drop superfluous fseek() from rea()

Make add require confirmation for unadvisable actions

Deleting a country in state STAT_SANCT, STAT_ACTIVE or STAT_GOD is
risky, because any references to this country become dangling, which
makes ef_verify() unhappy.  For a reason: we may well have code that
isn't prepared for dangling references, and breaks.

Replacing a country that is being used is risky, because it can get us
into weird states.  For instance, replacing a player by a visitor can
result in a visitor that owns stuff.

Make add refuse to touch a country while it's being played

I suspect player code could get terminally confused by country state
changing unexpectedly.  Not worth the risk.

Fix add not to crash on negative country number

Don't bother giving POGO BTUs in files

Deity's BTUs get reset to maximum on login anyway.

Create all deities with $123456789, not just POGO

Move setting nat_cnam, nat_pnam into nat_reset()

Use nat_reset() for POGO in files.c

Requires moving it from subs/natsub.c to common/nat.c.

Make files set the usual nation flags for POGO

The add command sets nation flags NF_FLASH, NF_BEEP, NF_COASTWATCH,
NF_SONAR, and since 4.2.6 NF_TECHLISTS.  POGO is created by the files
utility, which sets only NF_FLASH (since 4.2.2).  Change files to
match add.

nat_reset() is no longer used with STAT_SANCT, simplify

Change newcap back to not wiping the country

Just create sanctuaries, put country into STAT_SANCT, grant BTUs and
money, set origin and initial realms.

This reverts commit e1a68c72 (v4.3.12) as far as newcap is concerned.
Except we still set nat_access, because that needs to be set along
with nat_btu.

Additionally, leave levels and telegrams alone.

Should have no effect in practice, because newcap works only in
STAT_NEW, and we get there with the add command, which wipes the
country.

Make add always reset the country

Before, add reset the country only when adding a player or a visitor.
When adding a deity or deleting a country, it set just nat_cnam,
nat_pnam and nat_state.  Has always been that way.

Because of that, a newly minted deity country could inherit all kinds
of crap from a previous user of its country number: origin, realms,
relations, telegrams, ...  Harmless if the country number has never
been used before, which is how add is generally used.

When adding a deity country, initial levels (start_education, ...) now
apply, relations start NEUTRAL instead of AT_WAR, and the usual
initial nation flags are set.

Reset on delete as well, just to get rid of the special case.

Rename add argument "new" to "player"

This makes the correct choice for player countries obvious.  Keep
recognizing "new" for now, but deprecate it.

Drop add argument "active"

Argument "active" is obscure.  It creates a country in STAT_ACTIVE
that doesn't have a capital, and has its origin at the true origin.
If you really want such a country, create it in STAT_NEW normally,
then use edit to go to STAT_ACTIVE.

Drop add's obscure sector check and wipe functionality

If you really need to wipe out a country, there's much more to wipe
than just sectors.

Drop add's undocumented land unit destruction feature

It doesn't destroy ships, planes or nukes, either.

Permit no-op country name change again

Commit aa5861d1 (v4.3.20) made add, edit and change reject a country
name that is already in use.  Even if it's used by the same country.
Relax that.

Improve newcap's origin error message

Streamline a few files error messages

Write one nice line to stderr instead of a cryptic one to stderr, and
an uninformative one to stdout.

Fix files and fairland to report ef_open() failure just once

ef_open() already reports failure since commit 7eb2fe57, v4.2.20.
Drop the second report from its callers.

Replace exit(-1) by exit(1)

exit() uses only the least significant byte of its argument.

Make allocate_memory() return void

It can't fail since commit ba1a320c, v4.3.15.

Guard against unusable fd in pthreads' empth_select()

0 <= fd < FD_SETSIZE must hold, or else undefined behavior in
FD_SET().

Fix lwpSleepFd()'s guard against unusable fd

0 <= fd < FD_SETSIZE must hold, or else undefined behavior in FD_SET()
and buffer overrun in LwpFdwait[fd].  Check of upper bound off by one,
check of lower bound missing.

Plug FILE leak on show_first_tel() error paths

Leak introduced in commit 4238323d, v4.3.20.  Unlikely to bite in
practice, as only corrupt mailbox files can cause these errors.

Plug memory leak in set_paths() error path

Leak appeared in commit 91eefc3f, v4.3.12.  Harmless in practice,
since the error is ultimately fatal.

Fix read_schedule() not to fclose(stdin)

Did that since the initial commit 71320ed6, v4.3.10.

Plug FILE leak on read_schedule() parse error path

Leak introduced in initial commit 71320ed6, v4.3.10.

Plug memory leak in verify_row()

Leak introduced in commit b30c83cd, v4.3.28.

Make play show no last command when there hasn't been one

Before, it printed NULL, which isn't helpful.

Remove unreachable code in sell()

sell used to search multiple sectors for sellable commodities, keeping
tally in totalcom.  It failed with message "No eligible" when none
could be found.

sell's second argument got changed to a single sector in Empire 3.  If
the sector can't sell, we return early.  Else, totalcom is positive.
Thus, the "No eligible" code is unreachable.  Remove it.

Fix info sell on second argument

Empire 3 changed it to a single sector, but neglected to update info.

Make scripts/savecore create the core directory

Rename tgz to arc in scripts/backup

tgz looks silly when you hack the script to use something other than
tar+gzip.

Fix empire(6) synopsis for argument of -s

Commit 464094a6 (v4.3.26) changed it in section description, but
missed section synopsis.

Fix use-after-free when plane is downed or aborted in dogfight

ac_dog() passed ap and dp to ac_dog_report() after ac_planedamage()
freed it.  Broken in commit 73260a8a, v4.3.27.  Diagnosed with
valgrind.

Bump version to 4.3.29

Clean up omit_ids() not to set an extra element in idgap[]

Update change log for 4.3.28

Don't let POGO (#0) navigate and march ghosts

POGO can navigate dead ships, and march dead land units.  The ghosts
even get sighted and interdicted, and can hit mines (landmines only
until commit fe372539, v4.3.27).  Noted for ships in commit 9100af0b.
Has always been broken.  Fix by making shp_sel() and lnd_sel()
explicitly reject ghosts.

Same code pattern also exists in pln_sel, but dead plains fail the
efficiency test, so it's harmless there.  Apply the same fix anyway.

Make llook ignore land units and planes loaded on land units

Probably overlooked when trains were added in 4.0.0.

Fix "sector can board" test to ignore land units on land units

boar() lets a sector board if it has mobility or usable land units.
Embarked land units are not usable.  But it tests only "on ship", not
"on land unit".  Broken in 4.0.17.

Fix transport to reject planes loaded on land units

The transported plane remains on its carrier.  When the land unit
moves, the plane is teleported right back to it.  Broken since
Chainsaw 3 added land units.

While there, improve the message for planes on ships.

Fix lradar to reject land units loaded on land units

Probably overlooked when trains were added in 4.0.0.

While there, improve the message for land units on ships.

Fix strength to ignore land units loaded on land units

They can't defend their sector, see get_dlist().

Missed when 4.2.0 made them not defend the sector.

Fix navigate and march to find paths longer than 7 sectors again

Broken in commit 8f008bf8, v4.3.27.  How embarrassing...

Make march sub-command 'i' show military, #land units loaded

Update doc/debugging for recent commit a9611a57

New server option -F to force start even when state looks bad

Risks crashes and further corruption, but gives deities a chance to
fix up a bad game state with edit commands and such.

Give init_server() internal linkage

Change empdump to export even when game state looks bad

Just print a warning then.  Gives deities another tool to fix up a bad
game state.

Split ef_verify() into ef_verify_config(), ef_verify_state()

Factor verify_table() out of ef_verify()

Fix return value of ef_verify() to match function comment

Comment added in commit 6eec0010.  Callers don't actually care.

Fix prewrite callbacks' cargo list update for in-place update

When updating in-place (old==new), we must not write through new
before we're done reading the same memory through old.

Bug: we write the carrier uids too early.  Cargo lists aren't updated
when a carrier dies in an in-place update.  No such updates are known.
Broken in commit 64a53c90, v4.3.17.

Land units loaded on land units fight che again

They didn't since commit 93d033cf, v4.3.26.  Drawback: micromanagement
incentive to unload them for the update.  Similar incentive has always
existed for military on ships.

Since the previous commit, land units loaded on land units get
unloaded when the carrier dies fighting che.  Such land units get
stuck in the sector if the take over, and can be boarded.  Doesn't
feel right, and increases the micromanagement incentive.  Avoid by
letting them fight.

Don't leave cargo stuck on land unit killed by che

When che destroy a land unit, any embarked units remain stuck on their
now dead carrier.  Closely related to and same impact as the bug fixed
in commit 8ccad0d7.  Broken since Chainsaw 3 added land units.

The obvious fix would be to match what normally happens when a carrier
gets destroyed: destroy the cargo.  Requires recursion.  To keep
things as simple as possible, destroy plane cargo, but unload land
unit cargo.  That way, the only cargo of cargo to visit are nukes on
planes.

Unloading the land units creates another problem, which will be
addressed in the next commit.

Factor lnd_dies_fighting_che() out of take_casualties()

Units no longer die from lack of maintenance

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.

Only a deity can take units off a dead carrier.

Belatedly update info Maintenance for removed budget priorities

Missed in commit 520446ef, v4.3.6.

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 prexec() comment pasto

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.

Revert "Simplify execute(): use getstarg() instead of getstring()"

This reverts commit 96d1039c24774a9032b9e2014d4ce585fa0c764b.  It's in
the next commit's way.

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.

Don't record prompts in player_commands[]

Recording prompts is of marginal value, and it's in the next commit's
way.

If you need better logs after a crash, consider enabling the journal
log file.

Fix execute to print UTF-8 command correctly

execute() prints the UTF-8 command as normal text, which messes up
non-ASCII characters.  Broken in commit 2cc44bb1, v4.2.21.

Fix input/output filtering to ASCII

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.

Update and report status even after empty command

When getcommand() reads an empty string, it prints another prompt and
reads another line, without running status().  That's bad:

* nat_timeused is not updated.  Affects the player's prompt, deities'
  nat selector timeused, and commands edit, players, xdump nat.

* Mortal player isn't logged off for game hours, game down, and time
  limit.

* Notifications are delayed: going broke, becoming solvent, new
  telegrams (toggle inform off only), new announcements, capital lost.

Fix by removing the loop.  Callers handle empty input just fine since
4.2.2.

Side effect: empty commands are now recorded in player_commands[].
That's okay.

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 ""'.

Belatedly update xdump.pl not to use "xdump ver"

"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.

Disable test of pre_update_hook in nightly build

Where exactly the output from pre_update_hook gets inserted in the log
depends on timing.  Messes up diffs between test runs.

Fix nightly's diff_to_prev.sh

Broken in commit 5d512312, v4.3.11

Factor product code out of ef_verify() into verify_products()

Fold pln_zap_transient_flags() into verify_planes()

Verify carrier references are sane

Catch bad references here, before unit_cargo_init() chokes on them.