5 files changed, 61 insertions, 35 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index 73e24bcf829..a63e441dcb2 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,18 @@
 2013-07-19  Paul Eggert  <eggert@cs.ucla.edu>
 
+        Fix some minor file descriptor leaks and related glitches.
+        * filelock.c (create_lock_file) [!O_CLOEXEC]: Use fcntl with FD_CLOEXEC.
+        (create_lock_file): Use write, not emacs_write.
+        * image.c (slurp_file, png_load_body):
+        * process.c (Fnetwork_interface_list, Fnetwork_interface_info)
+        (server_accept_connection):
+        Don't leak an fd on memory allocation failure.
+        * image.c (slurp_file): Add a cheap heuristic for growing files.
+        * xfaces.c (Fx_load_color_file): Block input around the fopen too,
+        as that's what the other routines do.  Maybe input need not be
+        blocked at all, but it's better to be consistent.
+        Avoid undefined behavior when strlen is zero.
+
         * alloc.c (staticpro): Avoid buffer overrun on repeated calls.
         (NSTATICS): Now a constant; doesn't need to be a macro.
 
diff --git a/src/filelock.c b/src/filelock.c
index fefd14b3a92..b9c991e4baf 100644
--- a/src/filelock.c
+++ b/src/filelock.c
@@ -430,12 +430,14 @@ create_lock_file (char *lfname, char *lock_info_str, bool force)
       else
         {
           ptrdiff_t lock_info_len;
-#if ! HAVE_MKOSTEMP
+#if ! (HAVE_MKOSTEMP && O_CLOEXEC)
           fcntl (fd, F_SETFD, FD_CLOEXEC);
 #endif
           lock_info_len = strlen (lock_info_str);
           err = 0;
-          if (emacs_write (fd, lock_info_str, lock_info_len) != lock_info_len
+          /* Use 'write', not 'emacs_write', as garbage collection
+             might signal an error, which would leak FD.  */
+          if (write (fd, lock_info_str, lock_info_len) != lock_info_len
               || fchmod (fd, S_IRUSR | S_IRGRP | S_IROTH) != 0)
             err = errno;
           /* There is no need to call fsync here, as the contents of
diff --git a/src/image.c b/src/image.c
index 95d385dc9e2..1e3944ac1a1 100644
--- a/src/image.c
+++ b/src/image.c
@@ -2276,23 +2276,28 @@ slurp_file (char *file, ptrdiff_t *size)
   unsigned char *buf = NULL;
   struct stat st;
 
-  if (fp && fstat (fileno (fp), &st) == 0
-      && 0 <= st.st_size && st.st_size <= min (PTRDIFF_MAX, SIZE_MAX)
-      && (buf = xmalloc (st.st_size),
-          fread (buf, 1, st.st_size, fp) == st.st_size))
-    {
-      *size = st.st_size;
-      fclose (fp);
-    }
-  else
+  if (fp)
     {
-      if (fp)
-        fclose (fp);
-      if (buf)
+      ptrdiff_t count = SPECPDL_INDEX ();
+      record_unwind_protect_ptr (fclose_unwind, fp);
+
+      if (fstat (fileno (fp), &st) == 0
+          && 0 <= st.st_size && st.st_size < min (PTRDIFF_MAX, SIZE_MAX))
         {
-          xfree (buf);
-          buf = NULL;
+          /* Report an error if we read past the purported EOF.
+             This can happen if the file grows as we read it.  */
+          ptrdiff_t buflen = st.st_size;
+          buf = xmalloc (buflen + 1);
+          if (fread (buf, 1, buflen + 1, fp) == buflen)
+            *size = buflen;
+          else
+            {
+              xfree (buf);
+              buf = NULL;
+            }
         }
+
+      unbind_to (count, Qnil);
     }
 
   return buf;
@@ -5732,8 +5737,8 @@ png_load_body (struct frame *f, struct image *img, struct png_load_context *c)
       if (fread (sig, 1, sizeof sig, fp) != sizeof sig
           || fn_png_sig_cmp (sig, 0, sizeof sig))
         {
-          image_error ("Not a PNG file: `%s'", file, Qnil);
           fclose (fp);
+          image_error ("Not a PNG file: `%s'", file, Qnil);
           return 0;
         }
     }
diff --git a/src/process.c b/src/process.c
index 7c63964aee6..f4ae662468b 100644
--- a/src/process.c
+++ b/src/process.c
@@ -3526,10 +3526,13 @@ format; see the description of ADDRESS in `make-network-process'.  */)
   ptrdiff_t buf_size = 512;
   int s;
   Lisp_Object res;
+  ptrdiff_t count;
 
   s = socket (AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
   if (s < 0)
     return Qnil;
+  count = SPECPDL_INDEX ();
+  record_unwind_protect_int (close_file_unwind, s);
 
   do
     {
@@ -3545,9 +3548,7 @@ format; see the description of ADDRESS in `make-network-process'.  */)
     }
   while (ifconf.ifc_len == buf_size);
 
-  emacs_close (s);
-
-  res = Qnil;
+  res = unbind_to (count, Qnil);
   ifreq = ifconf.ifc_req;
   while ((char *) ifreq < (char *) ifconf.ifc_req + ifconf.ifc_len)
     {
@@ -3672,6 +3673,7 @@ FLAGS is the current flags of the interface.  */)
   Lisp_Object elt;
   int s;
   bool any = 0;
+  ptrdiff_t count;
 #if (! (defined SIOCGIFHWADDR && defined HAVE_STRUCT_IFREQ_IFR_HWADDR)  \
      && defined HAVE_GETIFADDRS && defined LLADDR)
   struct ifaddrs *ifap;
@@ -3686,6 +3688,8 @@ FLAGS is the current flags of the interface.  */)
   s = socket (AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
   if (s < 0)
     return Qnil;
+  count = SPECPDL_INDEX ();
+  record_unwind_protect_int (close_file_unwind, s);
 
   elt = Qnil;
 #if defined (SIOCGIFFLAGS) && defined (HAVE_STRUCT_IFREQ_IFR_FLAGS)
@@ -3802,9 +3806,7 @@ FLAGS is the current flags of the interface.  */)
 #endif
   res = Fcons (elt, res);
 
-  emacs_close (s);
-
-  return any ? res : Qnil;
+  return unbind_to (count, any ? res : Qnil);
 }
 #endif
 #endif  /* defined (HAVE_NET_IF_H) */
@@ -3978,6 +3980,7 @@ server_accept_connection (Lisp_Object server, int channel)
 #endif
   } saddr;
   socklen_t len = sizeof saddr;
+  ptrdiff_t count;
 
   s = accept4 (channel, &saddr.sa, &len, SOCK_CLOEXEC);
 
@@ -4000,6 +4003,9 @@ server_accept_connection (Lisp_Object server, int channel)
       return;
     }
 
+  count = SPECPDL_INDEX ();
+  record_unwind_protect_int (close_file_unwind, s);
+
   connect_counter++;
 
   /* Setup a new process to handle the connection.  */
@@ -4116,6 +4122,10 @@ server_accept_connection (Lisp_Object server, int channel)
   pset_filter (p, ps->filter);
   pset_command (p, Qnil);
   p->pid = 0;
+
+  /* Discard the unwind protect for closing S.  */
+  specpdl_ptr = specpdl + count;
+
   p->infd  = s;
   p->outfd = s;
   pset_status (p, Qrun);
diff --git a/src/xfaces.c b/src/xfaces.c
index d35851220b0..f647ff2e209 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -6283,6 +6283,7 @@ where R,G,B are numbers between 0 and 255 and name is an arbitrary string.  */)
   CHECK_STRING (filename);
   abspath = Fexpand_file_name (filename, Qnil);
 
+  block_input ();
   fp = emacs_fopen (SSDATA (abspath), "rt");
   if (fp)
     {
@@ -6290,29 +6291,24 @@ where R,G,B are numbers between 0 and 255 and name is an arbitrary string.  */)
       int red, green, blue;
       int num;
 
-      block_input ();
-
       while (fgets (buf, sizeof (buf), fp) != NULL) {
         if (sscanf (buf, "%u %u %u %n", &red, &green, &blue, &num) == 3)
           {
-            char *name = buf + num;
-            num = strlen (name) - 1;
-            if (num >= 0 && name[num] == '\n')
-              name[num] = 0;
-            cmap = Fcons (Fcons (build_string (name),
 #ifdef HAVE_NTGUI
-                                 make_number (RGB (red, green, blue))),
+            int color = RGB (red, green, blue);
 #else
-                                 make_number ((red << 16) | (green << 8) | blue)),
+            int color = (red << 16) | (green << 8) | blue;
 #endif
+            char *name = buf + num;
+            ptrdiff_t len = strlen (name);
+            len -= 0 < len && name[len - 1] == '\n';
+            cmap = Fcons (Fcons (make_string (name, len), make_number (color)),
                           cmap);
           }
       }
       fclose (fp);
-
-      unblock_input ();
     }
-
+  unblock_input ();
   return cmap;
 }
 #endif