xditem() needs a buffer that can hold entries of any xdumpable table.
It's been 2048 bytes and marked FIXME since day one. Clean it up so
that if anyone ever goes crazy with entry sizes, we fail an assertion
during startup instead of overrunning the buffer during play.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Disabled since commit 32fac04 (v4.2.13) because it could at the time
use more stack space than we provided. Additional issues: code still
uses obsolete gethostbyaddr() rather than getnameinfo(), and we
provide only 512 bytes for host names instead of the customary
NI_MAXHOST (1025) bytes.
All three issues would be easy enough to fix. What's not so easy is
to avoid blocking on the synchronous DNS lookup. Without that,
connecting repeatedly from a range of addresses with slow reverse
lookup could conceivably be employed as a denial of service attack.
We've been living without reverse lookup for close to ten years. Bury
the corpse, and move on.
Bonus: sizeof(struct natstr) is cut in half.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Its value has always been "", unless the deity recompiled with
RESOLVE_IPADDRESS defined, or imported another value with empdump.
I'm going to drop the RESOLVE_IPADDRESS code, including struct natstr
member nat_hostname. To prepare for that, fix the selector's value to
"", so it doesn't need nat_hostname, and deprecate it.
This changes its "len" in xdump meta nat from 512 to 0. Unlikely to
upset clients.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Can't just replace the literal 65536 by INT_MAX, because my system's
vfprintf() rejects that with EOVERFLOW.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
s + n is defined only as long as it points into or just beyond the
same array as s. Thus, our use of INT_MAX for "unlimited" relies on
undefined behavior. Works fine in practice, but avoiding undefined
pointer arithmetic isn't hard here, so let's do it.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
We're dealing with three kinds of string storage: char * pointing to a
null-terminated string, char[] holding a null-terminated string, and
char holding a string of length 0 or 1.
Unfortunately, xdump meta data doesn't distinguish the latter two:
both are NSC_STRINGY. Because of that, xundump happily fills char[]
to the limit, producing strings that aren't null-terminated, resulting
in read beyond buffer and possibly worse.
Affects struct shpstr members shp_path, shp_name, shp_rpath, struct
lndstr member lnd_rpath, and struct natstr members nat_cnam, nat_pnam,
nat_hostaddr, nat_hostname, nat_userid. Since these are all in game
state, only the empdump utility program is affected, not the
configuration table reader.
We clearly need to require null-termination for char[] values. Since
using char[1] for null-terminated strings makes no sense, we can still
make NSC_STRINGY with length 1 serve char values as before, by
permitting non-null-terminated strings only when length is 1. Ugly
wart, but it fixes the bug without a possibly awkward change xdump
meta-data.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
lnd_put() serves two masters: march, which wants it to report
"stopped" and write back struct ulist member mobility to
unit.land.lnd_mobil, and ground combat, which doesn't.
lnd_put() assumes march when actor is non-zero. Correct (but see
commit 8c502d4). Dates back to Empire 2.
Too ugly for my taste. Specialize for each master instead:
lnd_mar_put() for march, and lnd_put() for ground combat.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Commit d94d269 combined them into unit_put(), but that has turned out
not to be useful. Split them again.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
... get_dlist(), att_reacting_units().
This loses malloc() error checking in ask_olist() and get_dlist(). No
great loss, because we don't check in so many other places, including
att_reacting_units(). We should use a wrapper that terminates on
error, though. Left for another day.
ask_olist(), get_dlist() and att_reacting_units() zero the struct
ulist with memset(). lnd_insque() doesn't, so these functions need to
zero any members not otherwise initialized explicitly now.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
It's set to zero via memset(). Incorrect, but harmless, because it's
not actually used. Correct it anyway.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Members x, y, eff, supplied are not "LAND only", they're used only by
ground combat. They're not used by march.
In ground combat, member mobil is not how much the land unit has left,
it's how much it still needs to be charged.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Travis CI and OS X system make on 10.9.x at least don't have GNU make
>=3.82 which contains a parser enhancement that allows multiple
directives.
Signed-off-by: Gerd Flaig <gefla@pond.sub.org>
Here's why removing override is a good idea. The variable assignment
should already override anything Make may find in its environment.
All "override" does is protect against unwise make arguments.
"info make" says:
The `override' directive was not invented for escalation in the war
between makefiles and command arguments. It was invented so you can
alter and add to values that the user specifies with command
arguments.
Thus, override is ill-advised here.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Move clearing of retreat flags from retreat_ship(), retreat_land() to
retreat_ship1(), retreat_land1(), so it's where the retreat path is
shortened.
Move putship(), putland() from retreat_ship1(), retreat_land1() to
retreat_ship(), retreat_land(), so it's where the nxtitem() is, and
doesn't need a "if (!orig)" guard. Requires making retreat_ship1()
and retreat_land() return non-zero when they modified their argument.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When a retreating ship or land unit runs into a sector it can't enter,
it stops. The direction character that led it there is consumed, even
though it could not be followed. The next retreat will then attempt
to follow the rest of the path. Don't do that.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Undocumented misfeature: retreat and lretreat accept anything as
retreat path. The paths' actual consumers retreat_ship1() and
retread_land1() silently ignore invalid direction characters.
The retreat paths are in xdump, and invalid ones could conceivably
confuse smart clients.
Change the commands to reject invalid paths, and the consumers to oops
on invalid direction characters.
Note that invalid paths get rejected even when they're not actually
used because the conditions argument contains a "c" for "cancel".
Requiring the user give a new path so he can cancel the old one is
comically bad design.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Return RET_SYN instead of RET_FAIL then. Also drop the error message;
the usage help printed for RET_SYN should do.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Broken in commit bb5dfd8, v4.3.16. Fix by recognizing '?' only when
getting the argument interactively.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Obscure feature: 'h' in a retreat path stops the current retreat. The
code treats that as entering the current sector again, thus charges
mobility for staying put. It also reports "could not retreat to" for
a ship or land unit that can retreat out of, but could not retreat
into its current sector, e.g. a ship in an unfriendly harbor.
Fix by cleaning up the tortuous control flow.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The retreat code happily retreats anything, without considering who
owns it. It reports retreat to the owner by bulletin, even when the
owner is the current player.
Commands shouldn't report to the current player by bulletin, they
should print directly. Fixable. However, your ships and land units
retreating from your own actions makes little sense. Suppress it.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Code never actually retreated them, but it could zap their mission and
retreat flags. Harmless, but avoid it anyway.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Some callers have to second-guess shp_check_nav() to figure out
whether CN_LANDLOCKED means "too big to fit into the canal" or "can't
go there at all".
Fix that by returning d_navigation. CN_LANDLOCKED becomes either
NAV_CANAL or NAV_NONE, CN_CONSTRUCTION becomes either NAV_02 or
NAV_60, and CN_NAVIGABLE becomes NAVOK.
The CN_NAVIGABLE, ... codes are now unused. Drop them.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
A group retreat is executed in increasing UID order. The resulting
bulletin can be confusing.
Instead, retreat the ship that had its retreat conditions satisfied
first, and only then its group, if any.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The mission gets cleared whenever a retreat is triggered, even for
ships and land units that are unable to retreat.
Clear it only when the ship or land unit actually retreats.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
retreat_land() reads ships instead of land units, overrunning local
variable land. On lucky systems such as mine, this clobbers ni, and
triggers an oops. On unlucky systems, it crashes. On really unlucky
systems, it corrupts the land units file.
Broken since land unit retreat was added in Chainsaw 3.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Does not cover land unit retreat after a failed morale check.
The test exposes bugs. They're marked "BUG:" in the test input.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
"diff -q" isn't blessed by POSIX anyway. Neither is -u, but it should
be widely available. -c is blessed, but I find its output hard to
read.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
