]> granicus.if.org Git - nethack/commitdiff
[un]compress wait() feedback
authorPatR <rankin@nethack.org>
Sun, 19 Feb 2023 19:18:56 +0000 (11:18 -0800)
committerPatR <rankin@nethack.org>
Sun, 19 Feb 2023 19:18:56 +0000 (11:18 -0800)
At some point this may need to be commented out, but for the time
being report failure by wait() during the fork()+exec()+wait()
sequence when compressing or uncompressing a save file.

For the benefit of 'git log':  uncompressing an old save file or
compressing a new one fails when running nethack under control of
gdb (GNU Project's debugger) on OSX.  Noticed and reproducible on
OSX 10.11.6; no idea about more recent versions.  wait() returns -1
and sets errno to 4, "interrupted system call", and leaves the
variable that receives the child process's exit status as is.
NetHack has been relying on that exit status variable without
checking whether wait() returns success or failure.

This changes nethack's behavior when wait() fails.  It is now
intentionally failing compress/uncompress (by init'ing the status
to 1) after it was briefly treated as success (due to a recent
commit that set status to 0 before calling wait()), after a long
time of accidentally failing due to not setting varible 'i' (was 2
after being used for another purpose).  In addition to initializing
to failure prior to calling wait(), replace use of 'i' with a new
variable that is only used for this, possibly making the code more
comprehensible.

By the way, the issue with compress and uncompress failure when
running under control of gdb on OSX is not new.  I don't use gdb
a lot and have gotten tired of rediscovering this misbehavior.

src/files.c

index b3bb6e78bfc3a30f2ca1031ccfc7f3dc5ceda269..75c8ad40ab25486955939a2ec898e80d8b2c50af 100644 (file)
@@ -1359,10 +1359,10 @@ docompress_file(const char *filename, boolean uncomp)
     FILE *cf;
     const char *args[10];
 #ifdef COMPRESS_OPTIONS
-    char opts[80];
+    char opts[sizeof COMPRESS_OPTIONS];
 #endif
     int i = 0;
-    int f;
+    int f, childstatus;
     unsigned ln;
 #ifdef TTY_GRAPHICS
     boolean istty = WINDOWPORT(tty);
@@ -1461,12 +1461,28 @@ docompress_file(const char *filename, boolean uncomp)
         free((genericptr_t) cfn);
         return;
     }
+
+    /*
+     * back in parent...
+     */
 #ifndef NO_SIGNAL
-    i = 0; /* wait() will update this, hopefully just setting it back to 0 */
+    childstatus = 1; /* wait() should update this, ideally setting it to 0 */
     (void) signal(SIGINT, SIG_IGN);
     (void) signal(SIGQUIT, SIG_IGN);
-    /* wait() returns child's pid or -1, sets 'i' to child's exit status */
-    (void) wait((int *) &i);
+    errno = 0; /* avoid stale details if wait() doesn't set errno */
+    /* wait() returns child's pid and sets 'childstatus' to child's
+       exit status, or returns -1 and leaves 'childstatus' unmodified */
+    if ((long) wait((int *) &childstatus) == -1L) {
+        char numbuf[40];
+        const char *details = strerror(errno);
+
+        if (!details) {
+            Sprintf(numbuf, "(%d)", errno);
+            details = numbuf;
+        }
+        raw_printf("Wait when %scompressing %s failed; %s.",
+                   uncomp ? "un" : "", filename, details);
+    }
     (void) signal(SIGINT, (SIG_RET_TYPE) done1);
     if (wizard)
         (void) signal(SIGQUIT, SIG_DFL);
@@ -1477,9 +1493,9 @@ docompress_file(const char *filename, boolean uncomp)
      * compression if there are no signals, but we want this for
      * testing with FailSafeC
      */
-    i = 1;
+    childstatus = 1; /* non-zero => failure */
 #endif
-    if (i == 0) {
+    if (childstatus == 0) {
         /* (un)compress succeeded: remove file left behind */
         if (uncomp)
             (void) unlink(cfn);