navigate: Fix buffer overrun for impossibly long paths taken
When a player moves more than 1023 sectors in a single navigate
command, we overrun the buffer holding the path taken. Remote hole,
but it requires a ship that can go that far, and even a ship with
speed 1000 would need a tech level well in excess of 1000 for that.
Thus, the hole is purely theoretical for even remotely sane game
configurations.
First known version with the flaw is 4.0.0.
Fix by going back the older behavior: don't print the total path
taken, but do print what the path finder does. Context diff of an
example:
[0:634] Command : nav 3 6,0
Flagship is od oil derrick (#3)
+Using path 'n'
h =
k . .
j d
<67.2:67.2: 6,0> h
od oil derrick (#3) stopped at 6,0
-Path taken: n
This is how march works.
Removes the only use of shp_nav_one_sector()'s unusual return value 2.
Return 1 instead.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
empdump: Omit redundant data from export, new -c includes it
Cuts size of export files in test suite by a factor of four. Not a
big deal for disk usage, as export files compress very well, and disk
space is cheap anyway. Export files are simply easier to work with
when they aren't full of redundant crap.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The code needs it first so it can store field values into the object
right away. Save the values instead, and store them when the row is
complete.
The improvement is hardly worth the trouble by itself, but it'll
simplify supporting keys consisting of multiple fields, like sector
xloc, yloc or realm cnum, realm, so we can omit rows in those tables,
too.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
xundump: Permit omitting trailing sectors and realms
We currently require all rows to be present for tables item, sect-chr,
infrastructure, sect, realm.
The first three make sense: the code hard-codes indexes for them, and
malfunctions when entries are blank, so we want to make it hard to
leave any blank by accident.
The last two don't: blank sectors and realms work fine. There, the
restriction is arbitrary. Drop it.
Sectors and realms still can't be omitted "in the middle" (can do that
only with an ID selector), but that's coming soon.
Each part of a split table needs to supply rows for the same objects.
We currently require each part to name its objects explicitly, with an
object ID field, and don't support splitting tables that don't have
such IDs. These restrictions became arbitrary when commit 4e23c45
implemented checking each partial table supplies the same rows. Relax
them.
Array selectors were introduced for xdump (commit 612f2da, v4.2.18).
Since there is no syntax for subscripting arrays in conditions and
survey, we silently assume index zero. Unclean. Reject the selector
instead.
Affects ship selectors cargostart, cargoend, amtstart, amtend (since
commit 32011f1, v4.2.19), nat selectors relations (commit 0a44c48,
v4.3.0), contacts (commit c345ab8, v4.3.0), rejects (commit 6844c94,
v4.3.4). The other array selectors aren't visible in conditions. No
array selectors are visible in survey.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
nsc: Rename nstr_exec_val() to nstr_eval() and tighten contract
nstr_exec_val() can produce three different error values: NSC_NOTYPE
on invalid category, invalid type with zero val_as.lng on invalid type
(this is a bug), and the wanted type with zero val_s when it can't
coerce. None of these should ever happen.
Fix it to always produce an NSC_NOTYPE error value. Fix up callers to
check for it.
Specify the result's type is promoted on success. Ensure it is even
when the argument is NSC_VAL with an unpromoted type, which is
invalid.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
tests/files tests/fairland: Check stderr and exit status
New helper run_and_cmp to automate the job: run a program capturing
its standard output, standard error and exit status, then compare the
actual with the expected results.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Feeding other test output files (right now tests/files/files.out
tests/fairland/fairland.out fairland/newcap_script) works, but it's
odd, and might not work as well for all future output files. Only
strip trailing white space there.
There used to be other output files that required normalize.pl: client
output. Gone since commit 9ca3fa9.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
file: Provide EF_WITH_CADEF_MAX_ENTRY_SIZE to clean up xditem()
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>
player: Drop long-disabled code to resolve IP addresses
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>
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>
xundump: Refuse to undump strings too long for terminating null
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>
subs: Split lnd_mar_put() off lnd_put() and specialize
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>
subs: Factor lnd_insque() out of lnd_sel(), ask_olist(), ...
... 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>
Gerd Flaig [Fri, 2 Jan 2015 18:29:41 +0000 (19:29 +0100)]
Remove superfluous override directive in make check
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>
retreat: Clean up interface between retreat_FOO(), retreat_FOO1()
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>
retreat: Don't consume a retreat direction that wasn't followed
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>
retreat: Don't charge mobility for retreating in direction 'h'
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>
retreat: Don't retreat the current player's ships or land units
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>
shpsub: Make shp_check_nav() return more useful information
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>
retreat: Fix stack smash in land unit group retreat
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>
edit: Fix edit s key 'U' to preserve "does not follow"
Copying the ship copies the ship to follow. When the source ship
doesn't follow a ship, the target ship is made to follow the source.
Screwed up since Chainsaw added the means to copy a ship. Fix it.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
edit: Make edit l key 'L' preserve "no dist center"
Copying the sector copies its distribution center. When the source
sector has none, the target sector is made to distribute to the
source. Unexpected. Zap the distribution center then.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Setup uses edit to build units. Stupid idea, because that misses unit
initialization normally done by build, directly or via
unit_wipe_orders(). Use build instead.
Changes test output harmlessly: ship xbuilt, ybuilt go from 0,0 to the
building sector, ship#0's builder goes from 98 to 0, all ships'
cargostart and cargoend go from 0 to -1, jhb range from 0 to 35, and
land unit retreat percentage from 0 to 42.
Setup no longer needs country 98. Drop it.
Setup no longer copies 2,0 to 0,0 messing up its distribution center.
Harmless.
Setup doesn't need POGO's tech level anymore, so don't set it to 400.
Nukes are now built at their required tech level. Also harmless.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Let deities build in any sector. If the deity's tech level is too
low, use the required tech level instead. Don't require or use
materials, work or money. Bridge spans still require support.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
bridgefall: Fix loss of bridge support with EASY_BRIDGES off
With EASY_BRIDGES off, bridge spans need to be next to a bridge tower
or a bridge head that is at least 20% efficient to remain standing.
When a bridge tower or head gets damaged below 20%, adjacent spans may
lose support. Bug: they don't fall when they're next to another
bridge head below 20%.
Has always been broken.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
bridgefall: Fix support loss with EASY_BRIDGES and BRIDGETOWERS on
With EASY_BRIDGES on, bridge spans need to be next to land or a bridge
tower to remain standing.
Land can't go away, but a bridge tower can fall. Bridge spans next to
it may lose support then. Bug: they don't fall when they lose support
that way. Fix that.
for all possible pairs of directions (i, j)
if i and j cancel out
continue
do stuff
It does it by adding direction offsets to start coordinates, and
comparing the resulting coordinates to the start coordinates. Fine,
except it neglects to normalize the resulting coordinates.
Harmless in practice, because you can get an incorrect result only
when the path goes around the world, which it can do only in a 4x2
world.
Fix it anyway, by testing "directions cancel out" directly.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
projects / empserver / log