Replace the early returns in acurr() and acurrstr(), eliminating a
bunch of casts. I hope this doesn't reintroduce 'WIN32_BUG' (judging
by the previous workaround, I think that's extremely unlikely).
Also add an introductory comment to newhp().
Issue reported by Umbire: reviving a human corpse into a human
monster and then killing it entails murder penalty even when it is
hostile.
This is probably a non-issue. Human monsters tend to not leave
human corpses, they leave shopkeeper corpses or sergeant corpses
and so forth. Most human corpses created in normal play have
montraits attached and revive as a zombie, mummy, or vampire rather
than as a human.
This doesn't attempt to be clever, it just treats PM_HUMAN like
role monsters, not subject to 'murder'.
Closes#1180
Pull request from entrez: add support for 'make WANT_SOURCE_INSTALL=1'
to the linux.370 hints file, matching what macOS.370 already has.
I can't test for linux and don't bother with WANT_SOURCE_INSTALL on
OSX, so this hasn't been tried out.
Closes#1147
WANT_SOURCE_INSTALL=1 can be specified when compiling to build NetHack
within the repo/source tree, rather than in the normal system install
location. I find it useful for development on OSX, but it wasn't
present in any other hints files, so I've been using a custom hints file
on Linux for a while that adds the same option. The code is based on
the macOS.370 hints file.
Pull request from entrez: in the class filtering menu for multi-drop
and for loot in-or-out of a container, make choosing 'A' without any
other filter choices (such as all, specific class(es), cursed, unpaid,
just-picked-up, &c) become a no-op.
I started with the pull request and then undid much of it. It would
have been simpler to start from scratch. If you don't have option
paranoid_confirmation:AutoAll set, when choosing 'A' for all-without-
prompting as the only selection, operate as the PR has made things
work: effectively, 'A' by itself is ignored and the operation ends
with nothing happening.
However, if you do have paranoid_confirm:A set, then continue treating
'A' by itself as if 'A'+'a': everything. Since paranoid confirmation
is specified, that will be followed by a confirmation prompt.
This also adds a context-sensitive hint to the menu about how the 'A'
entry works, shown every time when 'cmdassist' is On or just once (per
session) when that is Off.
The documentation probably needs some updating.
Closes#1143
The 'A' option in the #loot or 'D'rop menu selecting every selectable
item when used on its own has been the cause of many bag of holding
explosions and other typo-related frustration. Now that it operates on
other filters rather than overriding them, actually require some other
filter be selected for it to have any effect. This means that 'A' on
its own will do nothing, but 'A'+'a' will still act like 'A' alone
previously did. I think this will reduce the rate of serious typo
accidents in games, without being intrusive and while still maintaining
'A' as a useful option (which I think it is, in it's 3.7 incarnation --
I use it all the time in combination with other filters, especially
justpicked).
Redo menu sizing to eliminate one of the analyzer hacks. There's no
need to loop through the menu entries a second time to find the last
one. The first time can remember it.
Also, some routines were taking arguments with
some_function(WINDOW *win, nhmenu *menu, ...)
and others with
other_function(nhmenu *menu, WINDOW *win, ...).
Change them all to have the window pointer 1st and menu pointer 2nd.
This is mostly just adding some Null guards ahead of
code that was already dereferencing pointers, so there
should be no change in behavior.
Also adds one validation of an array index that was
drawing a complaint.
"human", "dwarf", "elf", "gnome", and "orc" are all flagged M2_NOPOLY;
so is "giant". But dwarf and gnome are ordinary monsters and should
be eligible to be polymorph targets, so take the no-poly flag off of
them. The others are used for corpses and not intended to be distinct
monsters. But they are reasonable polymorph targets, so if player
with control picks any of them, choose a substitute. The exception is
human, which already has special poly-self handling.
add a static function maybe_destroy_armor() to replace
the DESTROY_ARM() macro.
The DESTROY_ARM() macro expanded to legal code, but did include
side-effects of making assignments to local variable otmp (not an
argument to the macro), referenced local variable atmp (not an
argument to the macro), and sometimes setting the in_use field
on the impacted armor obj.
The assignment statements within the if (...) caused some compilers
and code analyzers to complain and suggest that perhaps
'=' should have been '==', which was incorrect.
The maybe_destroy_armor() function provides the
caller with information about whether the armor resisted,
so that appropriate action can be taken within the caller.
The change 3 weeks ago for #4059 (to fix tty perm_invent memory leak)
resulted in nethack crashing during exit if built with TTY_PERM_INVENT
enabled and ending the game when perm_invent window isn't displayed.
This exits cleanly with or without perm_invent being shown and avoids
the memory leak which triggered the prior change.
src/restore.c(275): warning: Dereferencing NULL pointer. 'otmp' contains the
same NULL value as 'first' did.
src/restore.c(402): warning: Dereferencing NULL pointer. 'mtmp' contains the
same NULL value as 'first' did.
src/bones.c(646): warning: Using uninitialized memory 'oldbonesid'.
src/bones.c(646): warning: String 'oldbonesid' might not be zero-terminated.
Also help prevent a buffer overflow on corrupt or ill-formed bones.
The prior commit makes the code clearer so I'm not reverting it,
but the old code was not accessing freed memory so I am removing the
fixes entry. The static analyzer's complaint is bogus. Freeing the
memory that held a user-assigned type name did not affect whether
that pointer could be tested for being Null. Its stale value wasn't
being dereferenced.
docall() would access freed memory if the player used space(s) as
a fake object type name in order to remove an existing name without
giving any new one.
3.4.3 had this bug too; I didn't go farther back.
src/read.c(2889): warning: Dereferencing NULL pointer 'sobj'.
The analyzer doesn't know that the one caller that passes a NULL
sobj argument, angrygods(), checks !Punished before doing so.
Use a NULL guard before dereferencing sobj so that it doesn't
matter anyway.
The reason this wasn't caught sooner was because the version
of the Glyphinfo_at(x,y,glyph) macro, that was defined when
UNBUFFERED_GLYPHINFO is not defined (when glyphinfo's are
buffered - the default), does not use the 3rd argument at
all.
src/weapon.c(451): warning: Dereferencing NULL pointer 'uleft'.
The analyzer couldn't tell that a STRANGE_OBJECT not being made of
SILVER material, was sufficient to guard against dereference of
NULL uleft or uright in the l_ag and r_ag assignments.
Test and dereference each once to set indicator booleans, and use
the booleans afterwards.
New timed sanity checking trying to validate a timer's map location
can't locate a timed object (in this case, an egg with a hatch timer)
inside a container carried by a migrating monster.
The case of the object being carried directly by a migrating monster
was handled, but one inside a container wasn't.
src/wield.c(745): warning: Dereferencing NULL pointer 'obj'.
See line 685 for an earlier location where this can occur
In wield_tool(), the comparisons against uwep were intended
for when uwep wasn't null.
gcc/clang analyzers now have some awareness of obj arg being
notnull for wield_tool() since the extern.h prototypes
were changed to declare that, but other compilers/analyzers
do not necessarily have that information, and this:
'if (obj == uwep)'
would be a match if both were NULL.
src/wield.c(254): warning: Dereferencing NULL pointer 'wep'.
See line 190 for an earlier location where this can occur
This seems to be a case where an unnecessary null test (A) caused
the analyzer to call into question whether or not wep
is null at (B):
if (!wep) {
} else if (wep->otyp == CORPSE && cant_wield_corpse(wep)) {
} else if (uarms && bimanual(wep)) {
} else if (!retouch_object(&wep, FALSE)) {
} else {
/* Weapon WILL be wielded after this point */
if (will_weld(wep)) {
} else {
}
if (was_twoweap && !u.twoweap && flags.verbose) {
}
/* KMH -- Talking artifacts are finally implemented */
A ==> if (wep && wep->oartifact) {
}
if (artifact_light(wep) && !wep->lamplit) {
}
B ==> if (wep->unpaid) {
}
}
Removing the extraneous wep test from (A) resolves the complaint.
