With DEBUG suppressed, I started getting
16 warning: empty body in an if-statement
and 2 warning: empty body in an else-statement
from gcc.
Using braces for an empty block instead of just ';' avoids the warning:
if (foo)
debugpline("foo");
is bad,
if (bar) {
debugpline("bar");
}
is good. ;-)
The changes to lint.h are just precautionary.
modified:
include/lint.h
src/attrib.c, bones.c, dbridge.c, dig.c, eat.c,
makemon.c, mkmaze.c, mon.c, sp_lev.c
Shorten a function name in sp_lev.c that exceeded 31 characters.
That's a limit imposed by the VMS linker and the compiler complains
that it will be truncated.
Make all sp_lev.c functions which aren't listed in extern.h be static
and give all of them a declaration at the top of the file. I reordered
the ones already declared there in the same order as they occur in the
source, so the diff is quite a bit bigger than the actual changes.
(Once the one with the long name became static, the length of its name
no longer mattered, but I've shortened it anyway.)
Indent a couple of #pragma directives. Some pre-ANSI compiler didn't
like '#' in column 1 followed by something it didn't understand, even
when that occurred in a conditional block which was in the midst of
being excluded. (util/*_comp.y recently reminded me of that. files.c
should get a fix like this too.)
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....
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.
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.
Fix a couple of warnings about prototype not matching function definition.
Prototype specified a narrow scalar ('boolean') but function uses old-style
definition which implicitly promotes narrow types (char,short,'boolean' to
int, float to double). Switch prototypes to BOOLEAN_P.
A whole bunch of prototypes specify STATIC_DCL ('static') and then leave
the definition without STATIC_OVL (also 'static'). I only fixed up a
couple of those, but we risk getting somebody using an old compiler that
doesn't like
static int foo();
int foo() { return 42; }
Even if every comiler accepts that (I don't recall whether that was kosher
in the ancient K&R days), you can't tell by looking at the function
defintion (in the usual case where the forward declarations are at the top
of the file rather than immediately before the defintiion) whether it's
local to that file or global, so the missing STATIC_OVL qualifiers ought
to be added.
Bustling Town can be generated with inaccessible areas outside the
top edge of the fixed town map. If you end up in one of those area
without any way to dig or teleport, you're stuck.
This adds a new level flag "inaccessibles" to force checking for
such inaccessible areas, and add secret doors, holes/trapdoors,
or some random escape item into the areas.
His changes add file_exists() to sys/vms/vmsfiles.c, which might be
needed in the future but is not useful here. Checking for 'gbd' and
'grep' for PANICTRACE support (which I had working on Alpha--and
ought to work on VAX, no idea about IA64--before losing access to vms)
is pointless since neither is used.
src/files.c
SYSCF support, parse_config_line():
#if VMS, ignore GDBPATH and GREPPATH if present in sysconf;
SYSCF support, assure_syscf_file():
#if VMS, force open()'s optional third argument since the macro
which redirects open to vms_open requires it.
src/sp_lev.c
selection_do_randline():
use configuration-specified 'Rand()' rather than raw 'rand()'.
The special level loader has been using __FUNCTION__ in error messages
for a few months now, but that is a gcc extension (evidently picked up
by other compilers since only Borland had an issue so far). The
standard way to do the same thing is with __func__, but that's C99 so
we should avoid it. (__FUNCTION__ came earlier; gcc supports both.)
This switches to convential C code to achieve the same effect, using
the name 'nhFunc' rather than __FUNCTION__:
void foo()
{
static const char nhFunc[] = "foo";
... code that might report problem in nhFunc ...
return;
}
This has only been added to the functions which actually reference it,
not a blanket intrusion into every routine. In special level loader's
case, the reference is hidden in the opvar_free() macro which is used
quite a lot.
At first I used a macro:
void foo()
{
#define nhFunc "foo"
... code that might report problem in nhFunc ...
return;
#undef nhFunc
}
but using an actual variable avoids duplicate copies of the function
name string when used more than once inside a given function, and it
can't accidentally carry over into the next function due to missing or
misspelled #undef.
If we someday switch alloc() to give more specific information than
__FILE__, the macro variation would be better since the function name
won't be used most of the time (ie, when MONITOR_HEAP isn't defined).
Suppress some mostly longstanding "unused parameter" warnings where
the usage was generally conditional.
restlevl() had a conditional closing brace that confused the recent
reformat, resulting in some code inside a funciton ending up flush
against the left border (first column, that is, as if outside of the
function).
I'll push a formatting guide at some point. There may still be
outstanding changes, but please feel free to resolve those as you arrive
a them.
To the best of my knowledge, there is no changes to the actual code
content, but the formatter does have the occasional bug. If you run into
an issue, please fix it!
* remove several unreachable panic() calls; alloc() already panics
rather than return a null pointer;
* replace a couple of calls to malloc() with alloc() so that
MONITOR_HEAP won't produce spurious alloc/free discrepancies;
* replace several instances of variable 'd' with 'd0' so that it
doesn't conflict with the name of a function;
* comment out opvar_new_region() since it isn't used anywhere.
It should now be randomly disabled for a 3rd of Gehennom, to make things
a tad more interesting there. It's also disabled in Baalzebub's lair,
to make things a little more interesting.
Still don't know why the beetle is disappearing.