6 files changed, 260 insertions, 145 deletions

diff --git a/lib-src/ChangeLog b/lib-src/ChangeLog
index 8d2c95e671c..0bb24c375e7 100644
--- a/lib-src/ChangeLog
+++ b/lib-src/ChangeLog
@@ -1,3 +1,76 @@
+2015-03-27  Paul Eggert  <eggert@cs.ucla.edu>
+
+        Port etags to -DDEBUG
+        * etags.c (xnew, xrnew) [DEBUG]: Don't include chkmalloc.h, which
+        is not part of Emacs and is typically not installed.
+        Instead, just invoke xmalloc and xrealloc as usual.
+        Problem reported by Nicolas Richard in:
+        http://bugs.gnu.org/20191#20
+        (xrnew): Avoid no-longer-needed cast to 'char *'.
+        (xrealloc): First arg is now void *, not char *.
+
+2015-03-06  Paul Eggert  <eggert@cs.ucla.edu>
+
+        Random minor fixes for movemail
+        * movemail.c: Include <stdbool.h> and <signal.h>.
+        (waitpid) [WINDOWSNT]: New macro.
+        (wait) [WINDOWSNT]: Remove.
+        (main, popmail, pop_retr, mbx_write, mbx_delimit_begin)
+        (mbx_delimit_end): Use bool for boolean.
+        (main): Simplify #if usage a bit.
+        (main): Don't assume EOF == -1.  Prefer 'return' to 'exit'.  Don't
+        possibly unlink lockname twice, as that's a race condition.  Set
+        SIGCHLD to SIG_DFL to work around SysV misfeature.  Check for fork
+        failure.  Use waitpid, not wait, to avoid a race condition in the
+        unlikely case where we start up with a child.
+        (NOTOK, OK): Remove, in favor of plain boolean.
+        (popmail, pop_retr): Don't get confused about errno, e.g., ferror
+        need not set errno.
+        (popmail): Use fclose (mbf), not close (fileno (mbf)), to also
+        detect any stream-related errors (e.g., memory exhaustion).
+        (pop_retr): Report pop errors separately, since caller now does
+        errno reporting.
+        (mbx_write, mbx_delimit_begin, mbx_delimit_end): Check < 0, not ==
+        EOF, as it's a bit faster and (in theory) pickier.
+
+2015-02-27  Mark Laws  <mdl@60hz.org>
+
+        Support daemon mode on MS-Windows (bug#19688)
+        * emacsclient.c (decode_options) [WINDOWSNT]: Don't reject empty
+        arguments for --alternate-editor.
+        (print_help_and_exit) [WINDOWSNT]: Don't refrain from advertising
+        empty arguments for --alternate-editor.
+        (start_daemon_and_retry_set_socket) [WINDOWSNT]: MS-Windows
+        specific code to start Emacs in daemon mode and wait for it to be
+        ready for client connections.
+
+2015-02-23  Pete Williamson  <petewil0@googlemail.com>  (tiny change)
+
+        Use ${EXEEXT} more uniformly in makefiles
+        * Makefile.in (EMACS): Append ${EXEEXT}.
+
+2015-02-20  Paul Eggert  <eggert@cs.ucla.edu>
+
+        Simplify binary I/O configuration
+        * etags.c: Include <sysstdio.h> rather than <stdio.h>.
+        (process_file_name, analyze_regex): Use FOPEN_BINARY rather than
+        hard-coded "b".
+
+2015-02-19  Eli Zaretskii  <eliz@gnu.org>
+
+        * etags.c (process_file_name) [!DOS_NT]: Use "r", not "rb" in the
+        call to 'popen'.  (Bug#19735)
+
+2015-02-13  Paul Eggert  <eggert@cs.ucla.edu>
+
+        Better support for future plugins
+        See the thread containing:
+        http://lists.gnu.org/archive/html/emacs-devel/2015-02/msg00720.html
+        * make-docfile.c (write_globals): Generate code that #defines
+        Qxxx macros other than Qnil only if DEFINE_NONNIL_Q_SYMBOL_MACROS.
+        Qnil is safe to define even in plugins, since it must be zero for
+        other reasons.
+
 2015-01-24  Paul Eggert  <eggert@cs.ucla.edu>
 
         Fix a couple of AM_V_GEN bugs
diff --git a/lib-src/Makefile.in b/lib-src/Makefile.in
index d2705e7b5dc..6b5d379e38b 100644
--- a/lib-src/Makefile.in
+++ b/lib-src/Makefile.in
@@ -21,7 +21,7 @@
 SHELL = @SHELL@
 
 # Following ../lisp/Makefile.in.
-EMACS = ../src/emacs
+EMACS = ../src/emacs${EXEEXT}
 EMACSOPT = -batch --no-site-file --no-site-lisp
 
 # ==================== Things `configure' will edit ====================
diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index a04dda6408f..806275f5b1d 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -595,13 +595,6 @@ decode_options (int argc, char **argv)
       display = NULL;
       tty = 1;
     }
-
-  if (alternate_editor && alternate_editor[0] == '\0')
-    {
-      message (true, "--alternate-editor argument or ALTERNATE_EDITOR variable cannot be\n\
-an empty string");
-      exit (EXIT_FAILURE);
-    }
 #endif /* WINDOWSNT */
 }
 
@@ -642,10 +635,8 @@ The following OPTIONS are accepted:\n\
                         Set filename of the TCP authentication file\n\
 -a EDITOR, --alternate-editor=EDITOR\n\
                         Editor to fallback to if the server is not running\n"
-#ifndef WINDOWSNT
 "                       If EDITOR is the empty string, start Emacs in daemon\n\
                         mode and try connecting again\n"
-#endif /* not WINDOWSNT */
 "\n\
 Report bugs with M-x report-emacs-bug.\n");
   exit (EXIT_SUCCESS);
@@ -1511,7 +1502,77 @@ start_daemon_and_retry_set_socket (void)
       execvp ("emacs", d_argv);
       message (true, "%s: error starting emacs daemon\n", progname);
     }
-#endif /* WINDOWSNT */
+#else  /* WINDOWSNT */
+  DWORD wait_result;
+  HANDLE w32_daemon_event;
+  STARTUPINFO si;
+  PROCESS_INFORMATION pi;
+
+  ZeroMemory (&si, sizeof si);
+  si.cb = sizeof si;
+  ZeroMemory (&pi, sizeof pi);
+
+  /* We start Emacs in daemon mode, and then wait for it to signal us
+     it is ready to accept client connections, by asserting an event
+     whose name is known to the daemon (defined by nt/inc/ms-w32.h).  */
+
+  if (!CreateProcess (NULL, "emacs --daemon", NULL, NULL, FALSE,
+                      CREATE_NO_WINDOW, NULL, NULL, &si, &pi))
+    {
+      char* msg = NULL;
+
+      FormatMessage (FORMAT_MESSAGE_FROM_SYSTEM
+                     | FORMAT_MESSAGE_ALLOCATE_BUFFER
+                     | FORMAT_MESSAGE_ARGUMENT_ARRAY,
+                     NULL, GetLastError (), 0, (LPTSTR)&msg, 0, NULL);
+      message (true, "%s: error starting emacs daemon (%s)\n", progname, msg);
+      exit (EXIT_FAILURE);
+    }
+
+  w32_daemon_event = CreateEvent (NULL, TRUE, FALSE, W32_DAEMON_EVENT);
+  if (w32_daemon_event == NULL)
+    {
+      message (true, "Couldn't create Windows daemon event");
+      exit (EXIT_FAILURE);
+    }
+  if ((wait_result = WaitForSingleObject (w32_daemon_event, INFINITE))
+      != WAIT_OBJECT_0)
+    {
+      char *msg = NULL;
+
+      switch (wait_result)
+        {
+        case WAIT_ABANDONED:
+          msg = "The daemon exited unexpectedly";
+          break;
+        case WAIT_TIMEOUT:
+          /* Can't happen due to INFINITE.  */
+        default:
+        case WAIT_FAILED:
+          FormatMessage (FORMAT_MESSAGE_FROM_SYSTEM
+                         | FORMAT_MESSAGE_ALLOCATE_BUFFER
+                         | FORMAT_MESSAGE_ARGUMENT_ARRAY,
+                         NULL, GetLastError (), 0, (LPTSTR)&msg, 0, NULL);
+          break;
+        }
+      message (true, "Error: Could not start the Emacs daemon: %s\n", msg);
+      exit (EXIT_FAILURE);
+    }
+  CloseHandle (w32_daemon_event);
+
+  /* Try connecting, the daemon should have started by now.  */
+  /* It's just a progress message, so don't pop a dialog if this is
+     emacsclientw.  */
+  if (!w32_window_app ())
+    message (true,
+             "Emacs daemon should have started, trying to connect again\n");
+  if ((emacs_socket = set_socket (1)) == INVALID_SOCKET)
+    {
+      message (true,
+               "Error: Cannot connect even after starting the Emacs daemon\n");
+      exit (EXIT_FAILURE);
+    }
+#endif  /* WINDOWSNT */
 }
 
 int
diff --git a/lib-src/etags.c b/lib-src/etags.c
index dc2ced50933..b1361dbe7ad 100644
--- a/lib-src/etags.c
+++ b/lib-src/etags.c
@@ -122,7 +122,7 @@ char pot_etags_version[] = "@(#) pot revision number is 17.38.1.4";
 #include <stdarg.h>
 #include <stdlib.h>
 #include <string.h>
-#include <stdio.h>
+#include <sysstdio.h>
 #include <ctype.h>
 #include <errno.h>
 #include <sys/types.h>
@@ -176,17 +176,8 @@ char pot_etags_version[] = "@(#) pot revision number is 17.38.1.4";
  * SYNOPSIS:    Type *xnew (int n, Type);
  *              void xrnew (OldPointer, int n, Type);
  */
-#if DEBUG
-# include "chkmalloc.h"
-# define xnew(n,Type)     ((Type *) trace_malloc (__FILE__, __LINE__, \
-                                                  (n) * sizeof (Type)))
-# define xrnew(op,n,Type) ((op) = (Type *) trace_realloc (__FILE__, __LINE__, \
-                                        (char *) (op), (n) * sizeof (Type)))
-#else
-# define xnew(n,Type)     ((Type *) xmalloc ((n) * sizeof (Type)))
-# define xrnew(op,n,Type) ((op) = (Type *) xrealloc ( \
-                                        (char *) (op), (n) * sizeof (Type)))
-#endif
+#define xnew(n, Type)      ((Type *) xmalloc ((n) * sizeof (Type)))
+#define xrnew(op, n, Type) ((op) = (Type *) xrealloc (op, (n) * sizeof (Type)))
 
 typedef void Lang_function (FILE *);
 
@@ -348,7 +339,7 @@ static void canonicalize_filename (char *);
 static void linebuffer_init (linebuffer *);
 static void linebuffer_setlen (linebuffer *, int);
 static void *xmalloc (size_t);
-static void *xrealloc (char *, size_t);
+static void *xrealloc (void *, size_t);
 
 
 static char searchar = '/';     /* use /.../ searches */
@@ -1532,11 +1523,11 @@ process_file_name (char *file, language *lang)
   if (real_name == compressed_name)
     {
       char *cmd = concat (compr->command, " ", real_name);
-      inf = popen (cmd, "rb");
+      inf = popen (cmd, "r" FOPEN_BINARY);
       free (cmd);
     }
   else
-    inf = fopen (real_name, "rb");
+    inf = fopen (real_name, "r" FOPEN_BINARY);
   if (inf == NULL)
     {
       perror (real_name);
@@ -5600,7 +5591,7 @@ analyze_regex (char *regex_arg)
         char *regexfile = regex_arg + 1;
 
         /* regexfile is a file containing regexps, one per line. */
-        regexfp = fopen (regexfile, "rb");
+        regexfp = fopen (regexfile, "r" FOPEN_BINARY);
         if (regexfp == NULL)
           pfatal (regexfile);
         linebuffer_init (&regexbuf);
@@ -6533,7 +6524,7 @@ xmalloc (size_t size)
 }
 
 static void *
-xrealloc (char *ptr, size_t size)
+xrealloc (void *ptr, size_t size)
 {
   void *result = realloc (ptr, size);
   if (result == NULL)
diff --git a/lib-src/make-docfile.c b/lib-src/make-docfile.c
index 79d421a0a8e..bada8df9f72 100644
--- a/lib-src/make-docfile.c
+++ b/lib-src/make-docfile.c
@@ -707,12 +707,9 @@ write_globals (void)
                   globals[i].name, globals[i].name);
         }
       else if (globals[i].type == SYMBOL)
-        printf (("DEFINE_LISP_SYMBOL_BEGIN (%s)\n"
-                 "#define i%s %d\n"
-                 "#define %s builtin_lisp_symbol (i%s)\n"
-                 "DEFINE_LISP_SYMBOL_END (%s)\n\n"),
-                globals[i].name, globals[i].name, symnum++,
-                globals[i].name, globals[i].name, globals[i].name);
+        printf (("#define i%s %d\n"
+                 "DEFINE_LISP_SYMBOL (%s)\n"),
+                globals[i].name, symnum++, globals[i].name);
       else
         {
           if (globals[i].flags & DEFUN_noreturn)
@@ -740,15 +737,19 @@ write_globals (void)
   puts ("#ifdef DEFINE_SYMBOLS");
   puts ("static char const *const defsym_name[] = {");
   for (int i = 0; i < num_globals; i++)
-    {
-      if (globals[i].type == SYMBOL)
-        printf ("\t\"%s\",\n", globals[i].v.svalue);
-      while (i + 1 < num_globals
-             && strcmp (globals[i].name, globals[i + 1].name) == 0)
-        i++;
-    }
+    if (globals[i].type == SYMBOL)
+      printf ("\t\"%s\",\n", globals[i].v.svalue);
   puts ("};");
   puts ("#endif");
+
+  puts ("#define Qnil builtin_lisp_symbol (0)");
+  puts ("#if DEFINE_NON_NIL_Q_SYMBOL_MACROS");
+  num_symbols = 0;
+  for (int i = 0; i < num_globals; i++)
+    if (globals[i].type == SYMBOL && num_symbols++ != 0)
+      printf ("# define %s builtin_lisp_symbol (%d)\n",
+              globals[i].name, num_symbols - 1);
+  puts ("#endif");
 }
 
 
diff --git a/lib-src/movemail.c b/lib-src/movemail.c
index 5008c9b15f7..1618a6980e9 100644
--- a/lib-src/movemail.c
+++ b/lib-src/movemail.c
@@ -59,6 +59,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/file.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <errno.h>
 #include <time.h>
@@ -66,6 +67,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <getopt.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <signal.h>
 #include <string.h>
 #include "syswait.h"
 #ifdef MAIL_USE_POP
@@ -81,7 +83,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #undef access
 #undef unlink
 #define fork() 0
-#define wait(var) (*(var) = 0)
+#define waitpid(child, var, flags) (*(var) = 0)
 /* Unfortunately, Samba doesn't seem to properly lock Unix files even
    though the locking call succeeds (and indeed blocks local access from
    other NT programs).  If you have direct file access using an NFS
@@ -134,11 +136,11 @@ static void error (const char *s1, const char *s2, const char *s3);
 static _Noreturn void pfatal_with_name (char *name);
 static _Noreturn void pfatal_and_delete (char *name);
 #ifdef MAIL_USE_POP
-static int popmail (char *mailbox, char *outfile, int preserve, char *password, int reverse_order);
-static int pop_retr (popserver server, int msgno, FILE *arg);
-static int mbx_write (char *line, int len, FILE *mbf);
-static int mbx_delimit_begin (FILE *mbf);
-static int mbx_delimit_end (FILE *mbf);
+static int popmail (char *, char *, bool, char *, bool);
+static bool pop_retr (popserver, int, FILE *);
+static bool mbx_write (char *, int, FILE *);
+static bool mbx_delimit_begin (FILE *);
+static bool mbx_delimit_end (FILE *);
 #endif
 
 #if (defined MAIL_USE_MAILLOCK                                          \
@@ -166,23 +168,21 @@ main (int argc, char **argv)
   int indesc, outdesc;
   ssize_t nread;
   int wait_status;
-  int c, preserve_mail = 0;
+  int c;
+  bool preserve_mail = false;
 
 #ifndef MAIL_USE_SYSTEM_LOCK
   struct stat st;
   int tem;
-  char *lockname;
   char *tempname;
   size_t inname_len, inname_dirlen;
   int desc;
 #endif /* not MAIL_USE_SYSTEM_LOCK */
 
-#ifdef MAIL_USE_MAILLOCK
-  char *spool_name;
-#endif
+  char *spool_name = 0;
 
 #ifdef MAIL_USE_POP
-  int pop_reverse_order = 0;
+  bool pop_reverse_order = false;
 # define ARGSTR "pr"
 #else /* ! MAIL_USE_POP */
 # define ARGSTR "p"
@@ -193,19 +193,19 @@ main (int argc, char **argv)
 
   delete_lockname = 0;
 
-  while ((c = getopt (argc, argv, ARGSTR)) != EOF)
+  while (0 <= (c = getopt (argc, argv, ARGSTR)))
     {
       switch (c) {
 #ifdef MAIL_USE_POP
       case 'r':
-        pop_reverse_order = 1;
+        pop_reverse_order = true;
         break;
 #endif
       case 'p':
-        preserve_mail++;
+        preserve_mail = true;
         break;
       default:
-        exit (EXIT_FAILURE);
+        return EXIT_FAILURE;
       }
     }
 
@@ -223,7 +223,7 @@ main (int argc, char **argv)
 #else
       fprintf (stderr, "Usage: movemail [-p] inbox destfile%s\n", "");
 #endif
-      exit (EXIT_FAILURE);
+      return EXIT_FAILURE;
     }
 
   inname = argv[optind];
@@ -244,7 +244,7 @@ main (int argc, char **argv)
       status = popmail (inname + 3, outname, preserve_mail,
                         (argc - optind == 3) ? argv[optind+2] : NULL,
                         pop_reverse_order);
-      exit (status);
+      return status;
     }
 
   if (setuid (getuid ()) < 0)
@@ -253,18 +253,15 @@ main (int argc, char **argv)
 #endif /* MAIL_USE_POP */
 
 #ifndef DISABLE_DIRECT_ACCESS
+
+  char *lockname = 0;
+
 #ifndef MAIL_USE_MMDF
 #ifndef MAIL_USE_SYSTEM_LOCK
 #ifdef MAIL_USE_MAILLOCK
   spool_name = mail_spool_name (inname);
-  if (spool_name)
-    {
-#ifdef lint
-      lockname = 0;
-#endif
-    }
-  else
 #endif
+  if (! spool_name)
     {
       /* Use a lock file named after our first argument with .lock appended:
          If it exists, the mail file is locked.  */
@@ -292,7 +289,7 @@ main (int argc, char **argv)
         continue;
       tempname = xmalloc (inname_dirlen + sizeof "EXXXXXX");
 
-      while (1)
+      while (true)
         {
           /* Create the lock file, but not under the lock file name.  */
           /* Give up if cannot do that.  */
@@ -328,7 +325,10 @@ main (int argc, char **argv)
             {
               time_t now = time (0);
               if (st.st_ctime < now - 300)
-                unlink (lockname);
+                {
+                  unlink (lockname);
+                  lockname = 0;
+                }
             }
         }
 
@@ -337,15 +337,20 @@ main (int argc, char **argv)
 #endif /* not MAIL_USE_SYSTEM_LOCK */
 #endif /* not MAIL_USE_MMDF */
 
-  if (fork () == 0)
+#ifdef SIGCHLD
+  signal (SIGCHLD, SIG_DFL);
+#endif
+
+  pid_t child = fork ();
+  if (child < 0)
+    fatal ("Error in fork; %s", strerror (errno), 0);
+
+  if (child == 0)
     {
       int lockcount = 0;
       int status = 0;
 #if defined (MAIL_USE_MAILLOCK) && defined (HAVE_TOUCHLOCK)
-      time_t touched_lock;
-# ifdef lint
-      touched_lock = 0;
-# endif
+      time_t touched_lock IF_LINT (= 0);
 #endif
 
       if (setuid (getuid ()) < 0 || setregid (-1, real_gid) < 0)
@@ -382,9 +387,9 @@ main (int argc, char **argv)
 #ifdef MAIL_USE_MAILLOCK
       if (spool_name)
         {
-          /* The "0 - " is to make it a negative number if maillock returns
+          /* The "-" is to make it a negative number if maillock returns
              non-zero. */
-          status = 0 - maillock (spool_name, 1);
+          status = - maillock (spool_name, 1);
 #ifdef HAVE_TOUCHLOCK
           touched_lock = time (0);
 #endif
@@ -422,7 +427,7 @@ main (int argc, char **argv)
       {
         char buf[1024];
 
-        while (1)
+        while (true)
           {
             nread = read (indesc, buf, sizeof buf);
             if (nread < 0)
@@ -464,7 +469,7 @@ main (int argc, char **argv)
 #ifdef MAIL_USE_SYSTEM_LOCK
       if (! preserve_mail)
         {
-          if (ftruncate (indesc, 0L) != 0)
+          if (ftruncate (indesc, 0) != 0)
             pfatal_with_name (inname);
         }
 #endif /* MAIL_USE_SYSTEM_LOCK */
@@ -499,21 +504,18 @@ main (int argc, char **argv)
       if (spool_name)
         mailunlock ();
 #endif
-      exit (EXIT_SUCCESS);
+      return EXIT_SUCCESS;
     }
 
-  wait (&wait_status);
+  if (waitpid (child, &wait_status, 0) < 0)
+    fatal ("Error in waitpid; %s", strerror (errno), 0);
   if (!WIFEXITED (wait_status))
-    exit (EXIT_FAILURE);
+    return EXIT_FAILURE;
   else if (WEXITSTATUS (wait_status) != 0)
-    exit (WEXITSTATUS (wait_status));
+    return WEXITSTATUS (wait_status);
 
-#if !defined (MAIL_USE_MMDF) && !defined (MAIL_USE_SYSTEM_LOCK)
-#ifdef MAIL_USE_MAILLOCK
-  if (! spool_name)
-#endif /* MAIL_USE_MAILLOCK */
+  if (lockname)
     unlink (lockname);
-#endif /* not MAIL_USE_MMDF and not MAIL_USE_SYSTEM_LOCK */
 
 #endif /* ! DISABLE_DIRECT_ACCESS */
 
@@ -616,12 +618,6 @@ pfatal_and_delete (char *name)
 #include <pwd.h>
 #include <string.h>
 
-#define NOTOK (-1)
-#define OK 0
-
-static char Errmsg[200];        /* POP errors, at least, can exceed
-                                   the original length of 80.  */
-
 /*
  * The full valid syntax for a POP mailbox specification for movemail
  * is "po:username:hostname".  The ":hostname" is optional; if it is
@@ -637,10 +633,11 @@ static char Errmsg[200];	/* POP errors, at least, can exceed
  */
 
 static int
-popmail (char *mailbox, char *outfile, int preserve, char *password, int reverse_order)
+popmail (char *mailbox, char *outfile, bool preserve, char *password,
+         bool reverse_order)
 {
   int nmsgs, nbytes;
-  register int i;
+  int i;
   int mbfi;
   FILE *mbf;
   popserver server;
@@ -690,7 +687,8 @@ popmail (char *mailbox, char *outfile, int preserve, char *password, int reverse
         }
     }
 
-  if ((mbf = fdopen (mbfi, "wb")) == NULL)
+  mbf = fdopen (mbfi, "wb");
+  if (!mbf)
     {
       pop_close (server);
       error ("Error in fdopen: %s", strerror (errno), 0);
@@ -713,35 +711,28 @@ popmail (char *mailbox, char *outfile, int preserve, char *password, int reverse
     }
 
   for (i = start; i * increment <= end * increment; i += increment)
-    {
-      if (mbx_delimit_begin (mbf) != OK
-          || pop_retr (server, i, mbf) != OK)
-        {
-          error ("%s", Errmsg, 0);
-          close (mbfi);
-          return EXIT_FAILURE;
-        }
-      mbx_delimit_end (mbf);
-      fflush (mbf);
-      if (ferror (mbf))
-        {
-          error ("Error in fflush: %s", strerror (errno), 0);
-          pop_close (server);
-          close (mbfi);
-          return EXIT_FAILURE;
-        }
-    }
+    if (! (mbx_delimit_begin (mbf)
+           && pop_retr (server, i, mbf)
+           && mbx_delimit_end (mbf)
+           && fflush (mbf) == 0))
+      {
+        if (errno)
+          error ("Error in POP retrieving: %s", strerror (errno), 0);
+        pop_close (server);
+        fclose (mbf);
+        return EXIT_FAILURE;
+      }
 
   if (fsync (mbfi) != 0 && errno != EINVAL)
     {
       error ("Error in fsync: %s", strerror (errno), 0);
-      close (mbfi);
+      fclose (mbf);
       return EXIT_FAILURE;
     }
 
-  if (close (mbfi) != 0)
+  if (fclose (mbf) != 0)
     {
-      error ("Error in close: %s", strerror (errno), 0);
+      error ("Error in fclose: %s", strerror (errno), 0);
       return EXIT_FAILURE;
     }
 
@@ -765,7 +756,7 @@ popmail (char *mailbox, char *outfile, int preserve, char *password, int reverse
   return EXIT_SUCCESS;
 }
 
-static int
+static bool
 pop_retr (popserver server, int msgno, FILE *arg)
 {
   char *line;
@@ -773,8 +764,9 @@ pop_retr (popserver server, int msgno, FILE *arg)
 
   if (pop_retrieve_first (server, msgno, &line))
     {
-      snprintf (Errmsg, sizeof Errmsg, "Error from POP server: %s", pop_error);
-      return (NOTOK);
+      error ("Error from POP server: %s", pop_error, 0);
+      errno = 0;
+      return false;
     }
 
   while ((ret = pop_retrieve_next (server, &line)) >= 0)
@@ -782,24 +774,26 @@ pop_retr (popserver server, int msgno, FILE *arg)
       if (! line)
         break;
 
-      if (mbx_write (line, ret, arg) != OK)
+      if (! mbx_write (line, ret, arg))
         {
-          strcpy (Errmsg, strerror (errno));
+          int write_errno = errno;
           pop_close (server);
-          return (NOTOK);
+          errno = write_errno;
+          return false;
         }
     }
 
   if (ret)
     {
-      snprintf (Errmsg, sizeof Errmsg, "Error from POP server: %s", pop_error);
-      return (NOTOK);
+      error ("Error from POP server: %s", pop_error, 0);
+      errno = 0;
+      return false;
     }
 
-  return (OK);
+  return true;
 }
 
-static int
+static bool
 mbx_write (char *line, int len, FILE *mbf)
 {
 #ifdef MOVEMAIL_QUOTE_POP_FROM_LINES
@@ -811,47 +805,42 @@ mbx_write (char *line, int len, FILE *mbf)
                             && (a[4] == ' '))
   if (IS_FROM_LINE (line))
     {
-      if (fputc ('>', mbf) == EOF)
-        return (NOTOK);
+      if (fputc ('>', mbf) < 0)
+        return false;
     }
 #endif
   if (line[0] == '\037')
     {
-      if (fputs ("^_", mbf) == EOF)
-        return (NOTOK);
+      if (fputs ("^_", mbf) < 0)
+        return false;
       line++;
       len--;
     }
-  if (fwrite (line, 1, len, mbf) != len)
-    return (NOTOK);
-  if (fputc (0x0a, mbf) == EOF)
-    return (NOTOK);
-  return (OK);
+  return fwrite (line, 1, len, mbf) == len && 0 <= fputc ('\n', mbf);
 }
 
-static int
+static bool
 mbx_delimit_begin (FILE *mbf)
 {
   time_t now = time (NULL);
   struct tm *ltime = localtime (&now);
   if (!ltime)
-    return NOTOK;
+    return false;
 
   char fromline[100];
   if (! strftime (fromline, sizeof fromline,
                   "From movemail %a %b %e %T %Y\n", ltime))
-    return NOTOK;
-  if (fputs (fromline, mbf) == EOF)
-    return (NOTOK);
-  return (OK);
+    {
+      errno = EOVERFLOW;
+      return false;
+    }
+  return 0 <= fputs (fromline, mbf);
 }
 
-static int
+static bool
 mbx_delimit_end (FILE *mbf)
 {
-  if (putc ('\n', mbf) == EOF)
-    return (NOTOK);
-  return (OK);
+  return 0 <= putc ('\n', mbf);
 }
 
 #endif /* MAIL_USE_POP */