[Pkg-nethack-devel] Bug#203229: Bugs present in Debian releases of NetHack

Ben Gertzfield Ben Gertzfield <che@debian.org>, 203229@bugs.debian.org
Thu, 16 Oct 2003 00:13:29 -0700


--Apple-Mail-7--65620458
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed

On Oct 14, 2003, at 10:14 AM, Dave Cohrs wrote:

> Looking at some of the other bugs in the debian nethack list:
>
> #203229: Cannot save game correctly in discovery mode
>
> 	I cannot reproduce this using the official 3.4.2 binary for linux.
> 	Although I didn't go back and try older versions, I have to wonder
> 	if this is a bug in the way the debian binary was built or packaged
> 	and not something intrinsic to discovery mode.

Dave,

This is actually a pretty gross bug; we tracked it down and have 
attached a patch below.

Starting the game in explore mode (running nethack -X), on Unix at 
least, does not let you keep your save game correctly; it ends up 
changing the save game's permissions to 0, then compressing a *level* 
file -- not the save file -- when you answer 'y' to 'Do you want to 
keep the save file?'

The problem stems from the fact that when you use discovery or wizard 
mode, the game does not delete your save game right away.

Instead, it waits for sys/foo/foomain.c to prompt the user if they want 
to delete the game or not.

The mistake is that (for Unix's case) in sys/unix/unixmain.c, when 
getting the name of the save game file, we use buffnum 0 for fqname() 
instead of buffnum 1.

fqname's buffnum 0 seems to be a dumping ground for all sorts of 
temporary strings, and it changes quite often.  In this case, the code 
here in unixmain.c results in chmodding the save file to 0, then 
chmodding and gzipping the *level* file -- not the save file!

The save file is left at chmod 0, and can't be touched again, making it 
so the user can never save again without the admin manually removing 
the file.

                 const char *fq_save = fqname(SAVEF, SAVEPREFIX, 0);

                 (void) chmod(fq_save,0);        /* disallow parallel 
restores */

[...]

                 if (discover || wizard) {
                         if(yn("Do you want to keep the save file?") == 
'n')
                             (void) delete_savefile();
                         else {
                             (void) chmod(fq_save,FCMASK); /* back to 
readable */
                             compress(fq_save);
                         }
                 }

The fix I've come up with is somewhat ad-hoc, but works.  I found this 
comment in src/save.c:

src/save.c:     fq_save = fqname(SAVEF, SAVEPREFIX, 1); /* level files 
take 0 */

So as a guess, I changed unixmain.c:243's call to fqname() to pass in 
buffnum 1, not 0.  Et voila, the problem disappeared.

The problem is extremely reproducible, and I expect use of fqname() 
passing in buffnum 0 may cause other subtle problems in other systems.  
We should do a better audit of all the places that pass in buffnum 0 
and assume the string they get back won't change after they call a few 
other functions..

Patch against 3.4.2 attached.  This will be included in the Debian 
3.4.2-2 release.

As a last note, I noticed that answering "y" to the "Do you want to 
keep the save file?" question seems to echo "y" twice back to me.  It 
might be a Linux issue, but I wondered if it was easy to fix. 
(Answering "n" does not echo "n" twice, which is a little odd.)

Cheers,

Ben


--Apple-Mail-7--65620458
Content-Transfer-Encoding: 7bit
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="3.4.2_explore_save.patch"
Content-Disposition: attachment;
	filename=3.4.2_explore_save.patch

--- nethack-cvs/sys/unix/unixmain.c	Wed Oct 15 22:59:54 2003
+++ nethack-cvs-build/sys/unix/unixmain.c	Wed Oct 15 23:19:38 2003
@@ -243,7 +243,7 @@
 		 */
 		boolean remember_wiz_mode = wizard;
 #endif
-		const char *fq_save = fqname(SAVEF, SAVEPREFIX, 0);
+		const char *fq_save = fqname(SAVEF, SAVEPREFIX, 1); // level files use 0
 
 		(void) chmod(fq_save,0);	/* disallow parallel restores */
 		(void) signal(SIGINT, (SIG_RET_TYPE) done1);

--Apple-Mail-7--65620458
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed



--Apple-Mail-7--65620458--