Fix some more of the complaints from clang's static analyzer. The one
in options.c (manipulating warnings symbols) appears to be an actual bug.
All the rest are either because the analysis isn't quite sophicated
enough or outright bogus.
Two of them appear to be because a static routine is attempting to guard
against callers in the same file failing to pass in required output
pointers. Stripping away the check for missing pointer should convince
the analyzer that those output parameters always receive a value. We'll
see once the analysis is eventually re-run....
I think this should fix one of the valgrind complaints. Traps which
didn't use the trap->vl union field never initialized it, leaving a
bit of random garbage in the malloc'd trap structure. (And traps
which overwrote existing ones that did use it didn't reinitialize it
so kept stale data around.) Since those fields weren't in use by
the traps that don't care about them, this didn't provoke any actual
trouble.
Also reformatting....
This should avoid two of the three bogus clang complaints about
retaining the address of a stack variable after it has gone out of
scope.
Plus a recreation of some formatting I did a while back and then
accidentally clobbered before committing.
Fix a couple of the clang static analyzer's warnings.
muse.c has some reformatting. zap.c wasn't triggering any warning about
possible null pointer, but using MON_AT() to maybe avoid m_at() is not
a useful optimization since m_at() is a macro which starts out by using
MON_AT() itself.
When reading a novel, select a random passage which hasn't been shown
already. Once you've run through all the passages, it resets to get
them all again (with new random order that might happen to the be same
order if there aren't many passages). Switching to a different novel--
even another copy of the same one--will cause the previous passage
selection to be discarded and restarted from scratch if the prior book
is read again. Passage tracking for the most recently read novel is
kept across save and restore. (That means I needed to bump EDITLEVEL,
so it will need to be reset to 0 again before release.)
exit_nhwindows() is called before terminate(), and the tty incarnation
destroys all windows--including 'pickinv_cache_win'--without setting
the various index variables used to access them to WIN_ERR, then
terminate() calls freedynamicdata() which calls free_pickinv_cache()
which tries to destroy 'pickinv_cache_win' since it isn't WIN_ERR (if
the perm_invent option has been enabled during that playing session).
Some of the other <interface>_exit_nhwindows() also tear things down
without resetting the variables used to track them, so fixing this in
exit_nhwindows() would have been pretty messy.
Call free_pickinv_cache() before exit_nhwindows() in done(). At the
moment it's only called from done(), so other exit paths won't release
the small chunk(s) of memory used for the alternate inventory window
(if it got created for perm_invent support).
Replace the code that uses strcat with two pointers into the same buffer.
Treated separately, they point at distinct strings (no overlap possible),
but the C standard does disallow that in order to enable optimizations
using block transfer or such, so the tool that complained about it isn't
wrong. The characters getting appended to the output pointer can end
up overlapping the beginning of the other input pointer, conceivably
breaking an implementation that didn't use simple left-to-right byte-at-
a-time copying.
Also, I noticed that wishing for "luck stone" gave me a random gem.
There's code to strip off " stone" and compare against gem types, but it
prevents other code that accepts "foo bar" as a match for "foobar" and
vice versa from finding a match, since "luck" doesn't match anything
once "stone" is gone. So add the four gray stones into the array of
alternate spellings.
Another bit: it now accepts " gem" in addition to " stone" as optional
gem suffix, so "ruby", "ruby stone", and "ruby gem" all yield ruby.
("luck gem" won't work; you'll end up with a random gem-class object.)
And a last other bit: wishing for "lamp (lit) named foo" would yield
an unlit, unnamed lamp because "(lit)" followed by anything didn't match
"(lit)" and threw away everything past the opening paren. Now it will
produce "lamp named foo (lit)"--a lit lamp named "foo". (Wishing for
"lamp named foo (lit)" produces an unlit lamp named "foo (lit)". That's
acceptable to me... I'm not crawling any farther down this hole. Maybe
object formatting should be changed to keep the lit attribute in front
of the name?)
The memory leak (monst->mextra->edog, monst->mextra->mname,
monst->mextra for some monster were not released) I noticed recently
was due to recording a pet's full monster attributes with its corpse.
During save and restore, obj->oextra->omonst was being treated as a
full-fledged monster so worked as intended, but when freed, omonst
was treated as a black box and its mextra details weren't handled.
Changes to be committed:
modified: src/objects.c
modified: win/share/tilemap.c
Warnings during tile builds (and incorrect tile mappings
at run time when MAIL wasn't defined):
Creating 16x16 binary tile files (this may take some time)
warning: for tile 325 (numbered 325) of objects.txt,
found 'ETAOIN SHRDLU' while expecting 'stamped / mail'
warning: for tile 326 (numbered 326) of objects.txt,
found 'LOREM IPSUM' while expecting 'ETAOIN SHRDLU'
The recent addition of the first new extra scroll descriptions in a
very long time caused this problem to show up when MAIL was undefined.
There was a magic number in use that made an assumption that there
were only 4 such extra scroll descriptions, those being
"FOOBIE BLETCH", "TEMOV","GARVEN DEH","READ ME"
This isn't urgent, but I figure that until the mac build stuff gets
merged in, the core is still fair game....
'O' command's autopickup_exceptions was freeing a menu pick-list even
when it hadn't been allocated (for the list case, and for the remove
case if nothing was chosen for removal). That code was evidently used
as the model for msgtype and menucolors; they had the same situation.
I think ANSI and ISO sanction free(NULL) as a no-op, but pre-ANSI free
implementations don't necessarily handle that benignly. Even if they
all do, freeing something--even if that 'something' is nothing--which
hasn't been allocated is a bug on our end.
Replace several 'foo = alloc(strlen(bar)+1), strcpy(foo,bar)' sequences
with 'foo = dupstr(bar)' calls.
Change 'free(foo)' into 'free((genericptr_t) foo)' to possibly pacify
'lint' and/or really old compilers.
Add braces around 'if something;' when 'else { otherwise; }' has braces.
Simplify option value formatting for 'sortloot'.
Free sysopt.shellers and sysopt.explorers when releasing the memory used
for other sysopt fields.
Also some formatting stuff since sys.c was previously untouched.
This one has a couple of code changes included, but they shouldn't
produce any change in game play. If anyone adds a new shirt or shield
they'll have to update the corresponding foo_on() and foo_off() routines
to avoid an 'impossible' when putting on or taking off the new item(s),
the same situation as already happens for other subclasses of armor.
The automated reformatting put a space in casts of the form
'(type)(expression)', yielding '(type) (expression)', but it didn't
do that for '(typedef)(expression)'. There are lots of instances of
'(boolean)(expression)'; (uchar) and (xchar) also occur. I haven't
noticed other types, but I haven't looked in very many files yet.
Explicitly combine adjacent string literals so that pre-ANSI compilers
still have a chance to compile the code. I thought these had already
been dealt with, but I kept stumbling across them while reformatting,
so am trying to get them all out of the way now.
Fixing up mis-indented block comments, but hit some files that hadn't
had the earlier mixture of tab replacement, etc, so it's bigger than I
expected. If I get to it, they'll be another round of this tomorrow.
The intended change was that you'd get an increase to max energy if
current was sufficiently close to max rather than only when it was
already at max. It wasn't intended that you'd fail to have current
boosted all the way to max in the case where it wasn't sufficiently
close. That's fixed here.
Last few && or || followed by end-of-line comments, plus tab replacement
and 'return' parentheses. Not as many of those; some of these files had
already had that done.
Also, tweaked non-cursed scroll of charging read while confused to be a
tiny bit more effective.
To do: find and fix block comments that immediately follow a line with
an end-of-line comment and got misindented to line up with that comment.
The corridors used to head towards the goal using the straightest
possible line, often making a zig-zag beeline. Allow some slight variance,
sometimes going straight instead of turning, reducing the predictability,
and making those monotonously turning corridors less likely.
While the topic of strprepend() is current, make good use of it.
Simplify code which inserts "the Nth " in front of "<arrow,&c>".
I'm pretty sure there are one or two other places where I assumed that
the outpuf of xname() was a char array which is BUFSZ in length rather
than BUFSZ-PREFIX, and reused the buffer, but I don't know where they
occur. (BUFSZ-PREFIX is still big enough to hold most things, so it
might not lead to trouble.)
