Fix paid object on bill when angering another shopkeeper#8
Merged
Conversation
It's obviously supposed to be the latter and not the former. Interesting note: This same bug was found and fixed in NitroHack commit 4973ce4 (static checker day: fixes for scan-build and PVS warnings).
To test:
1. Get a level layout with two shops facing each other, e.g. minetn-4.
2. Sell a fragile object to one of the shops.
3. Dig a pit in the other shop's door space so its shopkeeper stays out
of the way.
4. Pick up an object in that other shop so it appears on your bill.
5. Zap a wand of striking at the first shop to break the fragile
object.
6. 'p'ay for the object picked up.
Expected result: Object gets the standard prompt to pay for it.
Actual result: "Paid object on bill??" followed by "Program in disorder
perhaps you'd better #quit." followed by the object being given to the
player for free.
The cause? This comment going all the way back to 2002:
> /* FIXME: object handling should be limited to
> items which are on this particular shk's bill */
Originally reported by PaRaD0xx in FreeNode's #NetHack IRC channel
whilst playing NAO343.
Based on DynaHack commit d995ed1 (Fix paid object on bill when angering
another shkp) by me.
feiyunw
pushed a commit
to feiyunw/NetHack
that referenced
this pull request
Jan 14, 2022
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
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.
This was referenced Jan 7, 2024
This was referenced Jun 3, 2024
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
g-branden-robinson
added a commit
to g-branden-robinson/NetHack
that referenced
this pull request
May 6, 2026
Unbreakable spaces in AT&T troff were always non-adjustable.
Define a string to use groff's `\~` extension if possible.
groff_man_style(7):
Portability
...
\~ Adjustable non‐breaking space. Use this escape sequence
to prevent a break inside a short phrase or between a
numerical quantity and its corresponding unit(s).
Before starting the motor,
set the output speed to\~1.
There are 1,024\~bytes in 1\~KiB.
CSTR\~NetHack#8 documents the B\~language.
\~ is a GNU extension also supported by Heirloom Doctools
troff 050915 (September 2005), mandoc 1.9.14
(2009‐11‐16), neatroff (commit 1c6ab0f6e, 2016‐09‐13),
and Plan 9 from User Space troff (commit 93f8143600,
2022‐08‐12), but not by DWB or Solaris troffs.
Fixes bad rendering in DWB 3.3 troff:
@@ -999 +999 @@
- file. -s|-s~-v may also be followed by arguments -p
+ file. -s|-s -v may also be followed by arguments -p
@@ -1005 +1005 @@
- entries which match both. -s|-s~-v may be followed by one
+ entries which match both. -s|-s -v may be followed by one
Solaris 10 troff _would_ misrender as well, but a different portability
problem keeps some of the foregoing text from rendering at all.
g-branden-robinson
added a commit
to g-branden-robinson/NetHack
that referenced
this pull request
May 6, 2026
Unbreakable spaces in AT&T troff were always non-adjustable.
Define a string to use groff's `\~` extension if possible.
groff_man_style(7):
Portability
...
\~ Adjustable non‐breaking space. Use this escape sequence
to prevent a break inside a short phrase or between a
numerical quantity and its corresponding unit(s).
Before starting the motor,
set the output speed to\~1.
There are 1,024\~bytes in 1\~KiB.
CSTR\~NetHack#8 documents the B\~language.
\~ is a GNU extension also supported by Heirloom Doctools
troff 050915 (September 2005), mandoc 1.9.14
(2009‐11‐16), neatroff (commit 1c6ab0f6e, 2016‐09‐13),
and Plan 9 from User Space troff (commit 93f8143600,
2022‐08‐12), but not by DWB or Solaris troffs.
Fixes bad rendering in DWB 3.3 troff:
@@ -999 +999 @@
- file. -s|-s~-v may also be followed by arguments -p
+ file. -s|-s -v may also be followed by arguments -p
@@ -1005 +1005 @@
- entries which match both. -s|-s~-v may be followed by one
+ entries which match both. -s|-s -v may be followed by one
Solaris 10 troff _would_ misrender as well, but a different portability
problem keeps some of the foregoing text from rendering at all.
SirWumpus
pushed a commit
to SirWumpus/NetHack
that referenced
this pull request
May 12, 2026
Unbreakable spaces in AT&T troff were always non-adjustable.
Define a string to use groff's `\~` extension if possible.
groff_man_style(7):
Portability
...
\~ Adjustable non‐breaking space. Use this escape sequence
to prevent a break inside a short phrase or between a
numerical quantity and its corresponding unit(s).
Before starting the motor,
set the output speed to\~1.
There are 1,024\~bytes in 1\~KiB.
CSTR\~NetHack#8 documents the B\~language.
\~ is a GNU extension also supported by Heirloom Doctools
troff 050915 (September 2005), mandoc 1.9.14
(2009‐11‐16), neatroff (commit 1c6ab0f6e, 2016‐09‐13),
and Plan 9 from User Space troff (commit 93f8143600,
2022‐08‐12), but not by DWB or Solaris troffs.
Fixes bad rendering in DWB 3.3 troff:
@@ -999 +999 @@
- file. -s|-s~-v may also be followed by arguments -p
+ file. -s|-s -v may also be followed by arguments -p
@@ -1005 +1005 @@
- entries which match both. -s|-s~-v may be followed by one
+ entries which match both. -s|-s -v may be followed by one
Solaris 10 troff _would_ misrender as well, but a different portability
problem keeps some of the foregoing text from rendering at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
To test:
Expected result: Object gets the standard prompt to pay for it.
Actual result: "Paid object on bill??" followed by "Program in disorder perhaps you'd better #quit." followed by the object being given to the player for free.
The cause? This comment going all the way back to 2002:
Originally reported by PaRaD0xx in FreeNode's #NetHack IRC channel whilst playing NAO343.
Based on DynaHack commit d995ed1 (Fix paid object on bill when angering another shkp) by me.