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: 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).
"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.
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.
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.
src/artifact.c(1589): warning C6011: Dereferencing NULL pointer 'magr'.
The 'struct monst *magr' parameter to artifact_hit() can be Null
if 'mdef' is youmonst. mdef is nonnull.
and having temporary stoning resistance timeout before finishing.
Issue reported by Umbire: hero was able to finish eating Medusa's
corpse safely after getting the message about no longer being
protected against stoning that is given when temporary resistance
times out.
The eating code was extending temporary resistance--when eating
something protected by such--to avoid just that. I thought this
was probably a message sequencing situation but it turns out that
the code was using touch_petrifies() to test the meal. It should
use flesh_petrifies() instead; Medusa doesn't pass touch_petrifies().
I didn't figure that out until after rewriting how the duration is
extended. The old way probably would have worked as desired with
the revised petrify test but I'm checking in the new version anyway.
Fixes#1186
If you wished for "lit candle" you'd get an unused candle that
is pre-lit but the feedback as it's added to inventory would be
"partly used candle (lit)". If snuffed out immediately, it reverts
to "candle" (ie, not partly used).
This fixes the first aspect: you will get "candle (lit)" added to
inventory. On the next turn it changes to partly used as expected.
The second aspect, reverting to not-used-yet after being lit during
the wish is left as-is.
src/role.c(1543): warning: Reading invalid data from 'roles'.
src/role.c(1765): warning: Reading invalid data from 'roles'.
src/role.c(1780): warning: Reading invalid data from 'races'.
Two variations:
IndexOk(idx, array) validate that idx is a valid index into the array
IndexOkT(idx, array) validate that idx is a valid index into the
array, excluding the final Terminator element
src/mkobj.c(419): warning: '((obj2))->oextra->omonst' could be '0'
: this does not adhere to the specification for the
function 'memcpy'.
src/mkobj.c(421): warning: Dereferencing NULL pointer
'((obj2))->oextra->omonst'.
See line 419 for an earlier location where this can occur
The analyzer was not aware that newoextra() sets up an oextra block:
if (!obj2->oextra)
obj2->oextra = newoextra();
The analyzer was also not aware that newomonst() was setting up a valid
OMONST pointer.
if (!OMONST(obj2))
newomonst(obj2);
Add an assert(has_omonst(obj2)) before copying the content from
OMONST(obj1) into OMONST(obj2).
src/mklev.c(137): warning: Using uninitialized memory 'ri'.
There was a for-loop assigning values to some elements of
ri[], but not all of them.
Initialize the array.
Four kinds of timers are defined but only two have ever been used.
Have sanity checking complain if the other two occur or if 'kind'
doesn't match any of the four.
Also, replacing a perfectly normal use of isok() with an inline test
just to pacify static analysis feels like a slippery slope, so handle
that a little differently.
I reordered the shrink_glob timer to put all object timers together.
Unfortunately that warrants incrementing EDITLEVEL which invalidates
existing save files.
