Skip to content

Return computed result instead of constant.#2

Merged
nhcopier merged 1 commit into
NetHack:NetHack-3.6.0from
UniQP:trap
Apr 13, 2016
Merged

Return computed result instead of constant.#2
nhcopier merged 1 commit into
NetHack:NetHack-3.6.0from
UniQP:trap

Conversation

@UniQP

@UniQP UniQP commented Mar 9, 2016

Copy link
Copy Markdown
Contributor

I did not tested the change (do not know how) but either "result" is unused or it should be returned. The patch tackles the latter case.

@nhcopier nhcopier merged commit 545455b into NetHack:NetHack-3.6.0 Apr 13, 2016
nhcopier pushed a commit that referenced this pull request May 18, 2016
Reported directly to devteam, zapping wand of undead turning at a
shopkeeper's corpse would cause a crash.  'Traits' to fully recreate
the shk were attached to the corpse, but the temporary monster
created on the map intended to be relaced by the shk didn't have any
eshk struct, and the sequence replmon() -> replshk() -> inhishop()
attempted to access mtmp->mextra->eshk when trying to reattach the
shk to his/her shop.  No other mextra structs involve pointer fixups,
so pets, priests, vault guards don't need extra handling.

I tested four cases.  #1 and #3 had no shop bill at the time; I'm not
sure about #2.  These all worked.
 1) shk killed inside shop, resurrected there;
 2) killed outside shop on the shop level, resurrected there;
 3) killed inside his shop, corpse carried to different level before
    being resurrected;
 4) killed and resurrected on different level from shop after hero
    stole something (teleported out of shop with unpaid item)--shk
    left shop to chase hero and followed him/her up some stairs.
k21971 added a commit to NHTangles/NetHack36 that referenced this pull request Oct 19, 2017
Reported directly to devteam, zapping wand of undead turning at a
shopkeeper's corpse would cause a crash.  'Traits' to fully recreate
the shk were attached to the corpse, but the temporary monster
created on the map intended to be relaced by the shk didn't have any
eshk struct, and the sequence replmon() -> replshk() -> inhishop()
attempted to access mtmp->mextra->eshk when trying to reattach the
shk to his/her shop.  No other mextra structs involve pointer fixups,
so pets, priests, vault guards don't need extra handling.

I tested four cases.  #1 and NetHack#3 had no shop bill at the time; I'm not
sure about NetHack#2.  These all worked.
 1) shk killed inside shop, resurrected there;
 2) killed outside shop on the shop level, resurrected there;
 3) killed inside his shop, corpse carried to different level before
    being resurrected;
 4) killed and resurrected on different level from shop after hero
    stole something (teleported out of shop with unpaid item)--shk
    left shop to chase hero and followed him/her up some stairs.

--PatR / commit 24400cd
nhmall added a commit that referenced this pull request Jun 29, 2019
nhcopier pushed a commit that referenced this pull request Apr 22, 2020
Remove an obsolete comment about soldiers since they haven't been
contiguous for over 15 years as the 'ants in barracks' bug report
revealed.

Do some reformatting, mostly for attacks among the '@' class.

Only one actual change:  reduce the level for 'elf' (placeholder
for zombie and mummy corpses) from 10 to 0 (and corresponding
difficulty from 12 to 2).  I don't think that level is ever used
anywhere and the one for humans is already zero.  Having it be
higher than Elvenking's level was absurd.

'@' section should either be completely reordered to obey 'rule #2'
or nurse should be moved back to where it once was (in front of
shopkeeper).  I haven't done either but only because I couldn't
which of the two should be done.
nhcopier pushed a commit that referenced this pull request May 29, 2020
Reported directly to devteam; transcription typo:  had "Vines", should
be "Vimes".

I double checked that long passage and found two other mistakes:
"proffered" was misspelled with 2nd 'r' doubled,
| 'Quoted statement,' someone said.  'Another statement.'
lacked the opening quote on the second sentence.

My source uses double quotes (normal American usage).  I'm not sure
why the passages which refer to it--the page number annotations
specify that same source--were transcribed with single quotes (normal
British usage), but I've left those as is.

The sentence with "Vines" was also separated from the previous one by
a single space when nearly everything in dat/tribute uses double space.
A quick regexp search found half a dozen or so other instances of that.
This fixes those but the searching wasn't rigorous and it's sometimes
ambiguous whether an elipsis or long dash constitutes the end of a
sentence before another starts or is just in the middle of a long one.
nhcopier pushed a commit that referenced this pull request Apr 2, 2021
Report raises two issues:
1) if you perform magic mapping while engulfed (or underwater) the
map got updated and player could view it with cursor+autodescribe,
but when done viewing it did not switch back to the limited engulfed
(or underwater) display.
2) when picking a teleport destination while engulfed/underwater you
have to pick the spot while seeing only the limited view of the map
that is shown while engulfed/underwater.

This fixes #1.  I'm inclined to classify #2 as traditional behavior
and am not going to try to figure out a fix for it.

Fixes #483
nhcopier pushed a commit that referenced this pull request Jul 13, 2021
Add three new passages, bringing total to 12.

Also, fix a typo in Thief of Time #2:  had "starts" where "stars"
was intended.
nhcopier pushed a commit that referenced this pull request Dec 1, 2022
If you were on a level teleporter, the spoteffects() call after
the hero gets expelled could end up going to a new level and
freeing all the monst chains from the level you were originally
engulfed on.

    #0 0xba0507 in free
    #1 0x87feda in dealloc_monst src/mon.c:2369
    #2 0x880a02 in dmonsfree src/mon.c:2194
    #3 0x9a7aa2 in savelev_core src/save.c:507
    #4 0x9a7a21 in savelev src/save.c:466
    #5 0x71eb9d in goto_level src/do.c:1483
    #6 0x71833f in deferred_goto src/do.c:1903
    #7 0xa2533f in level_tele src/teleport.c:1117
    #8 0xa2567b in level_tele_trap src/teleport.c:1198
    #9 0xa5c007 in trapeffect_level_telep src/trap.c:1861
    #10 0xa5f856 in trapeffect_selector src/trap.c:2497
    #11 0xa47497 in dotrap src/trap.c:2586
    #12 0x7d669b in spoteffects src/hack.c:2859
    #13 0x89d495 in xkilled src/mon.c:3187

The latter parts of xkilled() after the spoteffects() call would
then attempt to dereference the free'd monst pointer.

Save a copy of the monst struct prior to spoteffects() if you were
expelled, then point at the reference copy afterwards.

Resolves #938
nhcopier pushed a commit that referenced this pull request Jul 31, 2023
This is a re-creation of a project that was lost years ago while not
quite finished.  The old version included some instrumentation to
measure how many hits it takes to kill things during actual play; that
wasn't ready for prime time and this hasn't attempted to redo it.

Changes:
1) improves martial arts and bare-handed combat:  they now have a
   chance to hit twice when skill is better than 'basic'; 20% chance
   for second hit at skilled, 40% at expert, 60% at master, and 80% at
   grandmaster; when attacking more than once, strength bonus is
   handled as in #2;
2) nerfs two-weapon combat a bit:  hitting twice uses only 3/4 strength
   bonus on each hit, but when both attacks hit that's 3/2 bonus from
   strength which is still more than you get for one hit at a time;
3) beefs up two-handed weapons:  hitting via melee with a two-handed
   weapon uses 3/2 of stength bonus to reflect the increased influence
   of strength; isn't done for applied polearms though.

The reduction in strength bonus for two-weapon has far less impact
than it might sound, due to rounding up with the low values involved.
| full   3/4
|  +1 -> +1
|  +2 -> +2
|  +3 -> +2
|  +4 -> +3
|  +5 -> +4
|  +6 -> +5
The small reduction also doesn't matter if/when current hit happens to
deal a killing blow anyway.

Rings of increase damage apply at full value to every hit, same as
before.

When hitting bare-handed (#1 without gloves), a silver ring on either
hand continues to give a damage bonus against silver haters when you
make an ordinary single attack.  However if you attack twice, a silver
ring only applies on the first hit when it is worn on the right hand
and only applies on the second hit when worn on the left hand.  (Two
hits with a silver ring on each hand will give silver bonus for both.)

We might conceivably need to add support for a count prefix of 1 to
let player explicitly avoid a second bare-handed/martial-arts hit
attempt (similar to how throw and fire accept a count to limit missile
volley amount).

Kicking has been ignored.
nhcopier pushed a commit that referenced this pull request Aug 28, 2023
On GitHub issue 1090, Rhialto wrote:
Hi! I am currently in the process of adapting the pkgsrc packaging of NetHack 3.6.7 to include the curses window option. A bit late perhaps...

While working on that, I ran into a case of a NULL pointer being used. Apparently when linking with ncurses, this doesn't cause problems, but NetBSD's own curses doesn't like it, and crashes the game.

Here is the stack trace. It happens when #quitting, I think just before the high scores are about to be shown (but I guess it should show in the stack trace if I'm totally correct with that):

(gdb) bt
 #0  redrawwin (win=0x0) at /usr/src/lib/libcurses/touchwin.c:149
 #1  0x0000000017e104b4 in curses_refresh_nethack_windows () at ../win/curses/curswins.c:176
 #2  0x0000000017e1054a in curses_destroy_win (win=win@entry=0x7ad219520540) at ../win/curses/curswins.c:155
 #3  0x0000000017e1512c in curses_display_nhmenu (wid=<optimized out>, how=<optimized out>, _selected=0x7f7fff65d850) at ../win/curses/cursdial.c:771
 #4  0x0000000017e0ff81 in curses_select_menu (wid=wid@entry=21, how=how@entry=0, selected=selected@entry=0x7f7fff65d850) at ../win/curses/cursmain.c:607
 #5  0x0000000017e10257 in curses_display_nhwindow (wid=21, block=1) at ../win/curses/cursmain.c:385
 #6  0x0000000017ca8075 in really_done (how=<optimized out>, how@entry=13) at end.c:1609
 #7  0x0000000017ca993f in done (how=how@entry=13) at end.c:1201
 #8  0x0000000017ca9c85 in done2 () at end.c:381
 #9  done2 () at end.c:337
 #10 0x0000000017c489da in doextcmd () at cmd.c:363
 #11 0x0000000017c57e94 in rhack (cmd=<optimized out>, cmd@entry=0x0) at cmd.c:4909
 #12 0x0000000017c265da in moveloop (resuming=<optimized out>) at allmain.c:435
 #13 0x0000000017e1b862 in main (argc=<optimized out>, argv=<optimized out>) at ../sys/unix/unixmain.c:351
status_window is NULL here. Simply checking for NULL avoids the problem. I added checks for the other windows as well, while I was there.
mkuoppal added a commit to mkuoppal/NetHack that referenced this pull request Dec 5, 2023
If tutorial is entered, we get following leak on exit:

=================================================================
==81358==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 96 byte(s) in 3 object(s) allocated from:
    #0 0x7f6996edefdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
        NetHack#1 0x5601c255bcbb in alloc /home/miku/src/NetHack/src/alloc.c:71

Indirect leak of 5064 byte(s) in 3 object(s) allocated from:
    #0 0x7f6996edefdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
        NetHack#1 0x5601c255be1e in alloc /home/miku/src/NetHack/src/alloc.c:71
	    NetHack#2 0x5601c255be1e in dupstr /home/miku/src/NetHack/src/alloc.c:236

SUMMARY: AddressSanitizer: 5160 byte(s) leaked in 6 allocation(s).

Fix this by freeing the cloned selection before returning.
nhcopier pushed a commit that referenced this pull request Dec 17, 2023
If tutorial is entered, we get following leak on exit:

=================================================================
==81358==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 96 byte(s) in 3 object(s) allocated from:
    #0 0x7f6996edefdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
        #1 0x5601c255bcbb in alloc /home/miku/src/NetHack/src/alloc.c:71

Indirect leak of 5064 byte(s) in 3 object(s) allocated from:
    #0 0x7f6996edefdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
        #1 0x5601c255be1e in alloc /home/miku/src/NetHack/src/alloc.c:71
	    #2 0x5601c255be1e in dupstr /home/miku/src/NetHack/src/alloc.c:236

SUMMARY: AddressSanitizer: 5160 byte(s) leaked in 6 allocation(s).

Fix this by freeing the cloned selection before returning.
nhcopier pushed a commit that referenced this pull request Dec 23, 2024
If freedynamicdata() gets called twice, for whatever reason, a "double free" can occur.

warning: 44     ./nptl/pthread_kill.c: No such file or directory
(gdb) bt
#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=<optimized out>, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3  0x00007ffff7c8b26e in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x00007ffff7c6e8ff in __GI_abort () at ./stdlib/abort.c:79
#5  0x00007ffff7c6f7b6 in __libc_message_impl (fmt=fmt@entry=0x7ffff7e148d7 "%s\n")
    at ../sysdeps/posix/libc_fatal.c:132
#6  0x00007ffff7ceefe5 in malloc_printerr (str=str@entry=0x7ffff7e17bf0 "free(): double free detected in tcache 2")
    at ./malloc/malloc.c:5772
#7  0x00007ffff7cf154f in _int_free (av=0x7ffff7e49ac0 <main_arena>, p=<optimized out>, have_lock=0)
    at ./malloc/malloc.c:4541
#8  0x00007ffff7cf3d9e in __GI___libc_free (mem=0x555555ad82a0) at ./malloc/malloc.c:3398
#9  0x00005555557c12e9 in free_rect () at rect.c:48
#10 0x00005555557d77a2 in freedynamicdata () at save.c:1240
#11 0x0000555555682754 in nh_terminate (status=0) at end.c:1671
#12 0x000055555589af15 in opt_terminate () at ../sys/unix/unixmain.c:768
#13 0x000055555589af7a in after_opt_showpaths (dir=0x0) at ../sys/unix/unixmain.c:796
#14 0x0000555555693dd9 in do_deferred_showpaths (code=0) at files.c:4491
#15 0x0000555555778405 in initoptions () at options.c:6948
#16 0x0000555555899cd9 in main (argc=2, argv=0x7fffffffdad8) at ../sys/unix/unixmain.c:151
nhcopier pushed a commit that referenced this pull request Apr 29, 2025
The two options are very similar but probably mutually exclusive
except when using look-here and look-into-container (both via ':')
with the default setting for 'sortloot', or with inventory when
'sortpack' has been toggled off.

This removes 'use_menu_glyphs' and changes 'menu_objsyms' from a
boolean to a compound taking six possible values:
| 0: no object symbols in menus,
| 1: append object class symbol to object header lines (same as old
|menu_objsyms boolean),
| 2: include object symbol in menu entry lines for objects (same as
|recently added use_menu_glyphs),
| 3: both 1 and 2,
| 4: display as #2 but only if the menu lacks class header lines,
| 5: if header lines are present, display as #1; if headers are not
|present, then display as #4 (which will implicitly be #2).
Default is #4.

Effectively replaces the options portion of pull request #1406 and
retains the functionality, but not as default for normal menus.

Guidebook.tex is only partially updated.  Someone else will need to
finish that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants