bug#24218: 25.1.50; server-name, server-socket-dir and daemon start through, systemd socket

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

Matthew proposed a patch for this here:

https://lists.gnu.org/r/emacs-devel/2017-12/msg00903.html

which I am attaching in git form (see first attached patch). I also 
propose the second attached patch as a followup. It makes some of the 
newly-added features "private" (since they don't seem to be generally 
useful) and moves the low-level initialization code from emacs.c to 
process.c which is a more-natural home for it.


[-- Attachment #2: 0001-Fix-server-name-and-server-socket-dir-for-Bug-24218.patch --]
[-- Type: text/x-patch, Size: 8505 bytes --]

From 8debb5cd9ada5a8dc7d4a1e5300e7b913fe1e09b Mon Sep 17 00:00:00 2001
From: Matthew Leach <matthew@mattleach.net>
Date: Sun, 7 Jan 2018 13:47:12 -0800
Subject: [PATCH 1/2] Fix `server-name' and `server-socket-dir' for Bug#24218

* lisp/server.el: (server-external-socket-initialised): New
(server-name): Compute server name from `get-external-sockname'.
(server-socket-dir): Compute socket dir from
`get-external-sockname'.
(server-start): Don't check for existing server when an
uninitialised external socket has been passed to Emacs.
* src/emacs.c: (main): Obtain socket name via getsockname and pass
to `init_process_emacs'.
* src/lisp.h: (init_process_emacs): Add second parameter.
* src/process.c: (external_sock_name): New.
(get-external-sockname): New.
(init_process_emacs): Set `external_sock_name' to `sockname'
parameter.
---
 lisp/server.el | 56 ++++++++++++++++++++++++++++++++++++--------------------
 src/emacs.c    | 16 +++++++++++++---
 src/lisp.h     |  2 +-
 src/process.c  | 19 ++++++++++++++++++-
 4 files changed, 68 insertions(+), 25 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index ac0d701851..e8b53530c9 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -251,8 +251,16 @@ server-existing-buffer
 are done with it in the server.")
 (make-variable-buffer-local 'server-existing-buffer)
 
-;;;###autoload
-(defcustom server-name "server"
+(defvar server-external-socket-initialised nil
+  "When an external socket is passed into Emacs, we need to call
+`server-start' in order to initialise the connection.  This flag
+prevents multiple initialisations when an external socket has
+been consumed.")
+
+(defcustom server-name
+  (if (get-external-sockname)
+      (file-name-nondirectory (get-external-sockname))
+    "server")
   "The name of the Emacs server, if this Emacs process creates one.
 The command `server-start' makes use of this.  It should not be
 changed while a server is running."
@@ -263,8 +271,10 @@ server-name
 ;; We do not use `temporary-file-directory' here, because emacsclient
 ;; does not read the init file.
 (defvar server-socket-dir
-  (and (featurep 'make-network-process '(:family local))
-       (format "%s/emacs%d" (or (getenv "TMPDIR") "/tmp") (user-uid)))
+  (if (get-external-sockname)
+      (file-name-directory (get-external-sockname))
+    (and (featurep 'make-network-process '(:family local))
+         (format "%s/emacs%d" (or (getenv "TMPDIR") "/tmp") (user-uid))))
   "The directory in which to place the server socket.
 If local sockets are not supported, this is nil.")
 
@@ -618,23 +628,29 @@ server-start
       (when server-process
 	;; kill it dead!
 	(ignore-errors (delete-process server-process)))
-      ;; Delete the socket files made by previous server invocations.
-      (if (not (eq t (server-running-p server-name)))
-	  ;; Remove any leftover socket or authentication file
-	  (ignore-errors
-	    (let (delete-by-moving-to-trash)
-	      (delete-file server-file)))
-	(setq server-mode nil) ;; already set by the minor mode code
-	(display-warning
-	 'server
-	 (concat "Unable to start the Emacs server.\n"
-		 (format "There is an existing Emacs server, named %S.\n"
-			 server-name)
-		 (substitute-command-keys
-                  "To start the server in this Emacs process, stop the existing
+      ;; Check to see if an uninitialised external socket has been
+      ;; passed in, if that is the case, skip checking
+      ;; `server-running-p' as this will return the wrong result.
+      (if (and (get-external-sockname)
+               (not server-external-socket-initialised))
+          (setq server-external-socket-initialised t)
+        ;; Delete the socket files made by previous server invocations.
+        (if (not (eq t (server-running-p server-name)))
+           ;; Remove any leftover socket or authentication file
+           (ignore-errors
+             (let (delete-by-moving-to-trash)
+               (delete-file server-file)))
+         (setq server-mode nil) ;; already set by the minor mode code
+         (display-warning
+          'server
+          (concat "Unable to start the Emacs server.\n"
+                  (format "There is an existing Emacs server, named %S.\n"
+                          server-name)
+                  (substitute-command-keys
+                    "To start the server in this Emacs process, stop the existing
 server or call `\\[server-force-delete]' to forcibly disconnect it."))
-	 :warning)
-	(setq leave-dead t))
+          :warning)
+         (setq leave-dead t)))
       ;; If this Emacs already had a server, clear out associated status.
       (while server-clients
 	(server-delete-client (car server-clients)))
diff --git a/src/emacs.c b/src/emacs.c
index 20ced26283..1af09166b6 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -60,6 +60,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #ifdef HAVE_LIBSYSTEMD
 # include <systemd/sd-daemon.h>
 # include <sys/socket.h>
+# include <sys/un.h>
 #endif
 
 #ifdef HAVE_WINDOW_SYSTEM
@@ -1002,6 +1003,7 @@ main (int argc, char **argv)
 
 
   int sockfd = -1;
+  char *sockname = NULL;
 
   if (argmatch (argv, argc, "-fg-daemon", "--fg-daemon", 10, NULL, &skip_args)
       || argmatch (argv, argc, "-fg-daemon", "--fg-daemon", 10, &dname_arg, &skip_args))
@@ -1061,8 +1063,16 @@ main (int argc, char **argv)
 		  "Try 'Accept=false' in the Emacs socket unit file.\n"));
       else if (systemd_socket == 1
 	       && (0 < sd_is_socket (SD_LISTEN_FDS_START,
-				     AF_UNSPEC, SOCK_STREAM, 1)))
-	sockfd = SD_LISTEN_FDS_START;
+				    AF_UNIX, SOCK_STREAM, 1)))
+	{
+	  struct sockaddr_un sockaddr;
+	  socklen_t sockaddr_sz = sizeof(sockaddr);
+
+	  sockfd = SD_LISTEN_FDS_START;
+
+	  if (!getsockname(sockfd, &sockaddr, &sockaddr_sz))
+	    sockname = strdup(sockaddr.sun_path);
+	}
 #endif /* HAVE_LIBSYSTEMD */
 
 #ifdef USE_GTK
@@ -1656,7 +1666,7 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
   /* This can create a thread that may call getenv, so it must follow
      all calls to putenv and setenv.  Also, this sets up
      add_keyboard_wait_descriptor, which init_display uses.  */
-  init_process_emacs (sockfd);
+  init_process_emacs (sockfd, sockname);
 
   init_keyboard ();	/* This too must precede init_sys_modes.  */
   if (!noninteractive)
diff --git a/src/lisp.h b/src/lisp.h
index 3eb6e0d3c1..2e9002a226 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4313,7 +4313,7 @@ extern void delete_keyboard_wait_descriptor (int);
 extern void add_gpm_wait_descriptor (int);
 extern void delete_gpm_wait_descriptor (int);
 #endif
-extern void init_process_emacs (int);
+extern void init_process_emacs (int, char *);
 extern void syms_of_process (void);
 extern void setup_process_coding_systems (Lisp_Object);
 
diff --git a/src/process.c b/src/process.c
index d4440e472d..da8e5714b0 100644
--- a/src/process.c
+++ b/src/process.c
@@ -276,6 +276,10 @@ static int max_desc;
    the file descriptor of a socket that is already bound.  */
 static int external_sock_fd;
 
+/* The name (path) of the socket that was passed to Emacs, when
+   `external_sock_fd' is not -1.  */
+static const char *external_sock_name = NULL;
+
 /* Indexed by descriptor, gives the process (if any) for that descriptor.  */
 static Lisp_Object chan_process[FD_SETSIZE];
 static void wait_for_socket_fds (Lisp_Object, char const *);
@@ -7974,10 +7978,21 @@ restore_nofile_limit (void)
 }
 
 \f
+DEFUN ("get-external-sockname", Fget_external_sockname, Sget_external_sockname, 0, 0, 0,
+       doc: /* Return the path of an external socket passed to Emacs.
+Otherwise return nil.  */)
+     (void)
+{
+    if (external_sock_name)
+        return make_string(external_sock_name, strlen(external_sock_name));
+    else
+        return Qnil;
+}
+
 /* This is not called "init_process" because that is the name of a
    Mach system call, so it would cause problems on Darwin systems.  */
 void
-init_process_emacs (int sockfd)
+init_process_emacs (int sockfd, char *sockname)
 {
 #ifdef subprocesses
   int i;
@@ -8012,6 +8027,7 @@ init_process_emacs (int sockfd)
 #endif
 
   external_sock_fd = sockfd;
+  external_sock_name = sockname;
   max_desc = -1;
   memset (fd_callback_info, 0, sizeof (fd_callback_info));
 
@@ -8306,4 +8322,5 @@ returns non-`nil'.  */);
   defsubr (&Sprocess_inherit_coding_system_flag);
   defsubr (&Slist_system_processes);
   defsubr (&Sprocess_attributes);
+  defsubr (&Sget_external_sockname);
 }
-- 
2.14.3


[-- Attachment #3: 0002-Minor-cleanups-for-server-name-fix-Bug-24218.patch --]
[-- Type: text/x-patch, Size: 9315 bytes --]

From 51e1d2919b261be0c734ab3a19b3478fb52f43c1 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 7 Jan 2018 13:47:12 -0800
Subject: [PATCH 2/2] Minor cleanups for server-name fix (Bug#24218)

* lisp/server.el (server--external-socket-initialized): Rename
from server-external-socket-initialised, since it should be
private and Emacs uses American spelling.  All uses changed.
* src/emacs.c, src/lisp.h: Revert previous changes, as the
initialization is now done in src/process.c, which already
includes the relevant files.
* src/process.c (union u_sockaddr): Move decl to top level.
(external_sock_name, Fget_external_sockname): Remove, replacing
with Vinternal__external_sockname.  All uses changed.
(init_process_emacs): Deduce socket name ourselves rather than
have main.c do it.  Use conv_sockaddr_to_lisp instead of doing
it by hand.  Conditionalize it on HAVE_GETSOCKNAME.
---
 lisp/server.el | 24 ++++++++++++------------
 src/emacs.c    | 16 +++-------------
 src/lisp.h     |  2 +-
 src/process.c  | 58 ++++++++++++++++++++++++++++++----------------------------
 4 files changed, 46 insertions(+), 54 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index e8b53530c9..c867fde832 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -251,15 +251,15 @@ server-existing-buffer
 are done with it in the server.")
 (make-variable-buffer-local 'server-existing-buffer)
 
-(defvar server-external-socket-initialised nil
+(defvar server--external-socket-initialized nil
   "When an external socket is passed into Emacs, we need to call
-`server-start' in order to initialise the connection.  This flag
-prevents multiple initialisations when an external socket has
+`server-start' in order to initialize the connection.  This flag
+prevents multiple initializations when an external socket has
 been consumed.")
 
 (defcustom server-name
-  (if (get-external-sockname)
-      (file-name-nondirectory (get-external-sockname))
+  (if internal--external-sockname
+      (file-name-nondirectory internal--external-sockname)
     "server")
   "The name of the Emacs server, if this Emacs process creates one.
 The command `server-start' makes use of this.  It should not be
@@ -271,8 +271,8 @@ server-name
 ;; We do not use `temporary-file-directory' here, because emacsclient
 ;; does not read the init file.
 (defvar server-socket-dir
-  (if (get-external-sockname)
-      (file-name-directory (get-external-sockname))
+  (if internal--external-sockname
+      (file-name-directory internal--external-sockname)
     (and (featurep 'make-network-process '(:family local))
          (format "%s/emacs%d" (or (getenv "TMPDIR") "/tmp") (user-uid))))
   "The directory in which to place the server socket.
@@ -628,15 +628,15 @@ server-start
       (when server-process
 	;; kill it dead!
 	(ignore-errors (delete-process server-process)))
-      ;; Check to see if an uninitialised external socket has been
+      ;; Check to see if an uninitialized external socket has been
       ;; passed in, if that is the case, skip checking
       ;; `server-running-p' as this will return the wrong result.
-      (if (and (get-external-sockname)
-               (not server-external-socket-initialised))
-          (setq server-external-socket-initialised t)
+      (if (and internal--external-sockname
+               (not server--external-socket-initialized))
+          (setq server--external-socket-initialized t)
         ;; Delete the socket files made by previous server invocations.
         (if (not (eq t (server-running-p server-name)))
-           ;; Remove any leftover socket or authentication file
+           ;; Remove any leftover socket or authentication file.
            (ignore-errors
              (let (delete-by-moving-to-trash)
                (delete-file server-file)))
diff --git a/src/emacs.c b/src/emacs.c
index 1af09166b6..20ced26283 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -60,7 +60,6 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #ifdef HAVE_LIBSYSTEMD
 # include <systemd/sd-daemon.h>
 # include <sys/socket.h>
-# include <sys/un.h>
 #endif
 
 #ifdef HAVE_WINDOW_SYSTEM
@@ -1003,7 +1002,6 @@ main (int argc, char **argv)
 
 
   int sockfd = -1;
-  char *sockname = NULL;
 
   if (argmatch (argv, argc, "-fg-daemon", "--fg-daemon", 10, NULL, &skip_args)
       || argmatch (argv, argc, "-fg-daemon", "--fg-daemon", 10, &dname_arg, &skip_args))
@@ -1063,16 +1061,8 @@ main (int argc, char **argv)
 		  "Try 'Accept=false' in the Emacs socket unit file.\n"));
       else if (systemd_socket == 1
 	       && (0 < sd_is_socket (SD_LISTEN_FDS_START,
-				    AF_UNIX, SOCK_STREAM, 1)))
-	{
-	  struct sockaddr_un sockaddr;
-	  socklen_t sockaddr_sz = sizeof(sockaddr);
-
-	  sockfd = SD_LISTEN_FDS_START;
-
-	  if (!getsockname(sockfd, &sockaddr, &sockaddr_sz))
-	    sockname = strdup(sockaddr.sun_path);
-	}
+				     AF_UNSPEC, SOCK_STREAM, 1)))
+	sockfd = SD_LISTEN_FDS_START;
 #endif /* HAVE_LIBSYSTEMD */
 
 #ifdef USE_GTK
@@ -1666,7 +1656,7 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
   /* This can create a thread that may call getenv, so it must follow
      all calls to putenv and setenv.  Also, this sets up
      add_keyboard_wait_descriptor, which init_display uses.  */
-  init_process_emacs (sockfd, sockname);
+  init_process_emacs (sockfd);
 
   init_keyboard ();	/* This too must precede init_sys_modes.  */
   if (!noninteractive)
diff --git a/src/lisp.h b/src/lisp.h
index 2e9002a226..3eb6e0d3c1 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4313,7 +4313,7 @@ extern void delete_keyboard_wait_descriptor (int);
 extern void add_gpm_wait_descriptor (int);
 extern void delete_gpm_wait_descriptor (int);
 #endif
-extern void init_process_emacs (int, char *);
+extern void init_process_emacs (int);
 extern void syms_of_process (void);
 extern void setup_process_coding_systems (Lisp_Object);
 
diff --git a/src/process.c b/src/process.c
index da8e5714b0..2613dd2be0 100644
--- a/src/process.c
+++ b/src/process.c
@@ -160,6 +160,18 @@ static bool kbd_is_on_hold;
    when exiting.  */
 bool inhibit_sentinels;
 
+union u_sockaddr
+{
+  struct sockaddr sa;
+  struct sockaddr_in in;
+#ifdef AF_INET6
+  struct sockaddr_in6 in6;
+#endif
+#ifdef HAVE_LOCAL_SOCKETS
+  struct sockaddr_un un;
+#endif
+};
+
 #ifdef subprocesses
 
 #ifndef SOCK_CLOEXEC
@@ -276,10 +288,6 @@ static int max_desc;
    the file descriptor of a socket that is already bound.  */
 static int external_sock_fd;
 
-/* The name (path) of the socket that was passed to Emacs, when
-   `external_sock_fd' is not -1.  */
-static const char *external_sock_name = NULL;
-
 /* Indexed by descriptor, gives the process (if any) for that descriptor.  */
 static Lisp_Object chan_process[FD_SETSIZE];
 static void wait_for_socket_fds (Lisp_Object, char const *);
@@ -4679,16 +4687,7 @@ server_accept_connection (Lisp_Object server, int channel)
   struct Lisp_Process *ps = XPROCESS (server);
   struct Lisp_Process *p;
   int s;
-  union u_sockaddr {
-    struct sockaddr sa;
-    struct sockaddr_in in;
-#ifdef AF_INET6
-    struct sockaddr_in6 in6;
-#endif
-#ifdef HAVE_LOCAL_SOCKETS
-    struct sockaddr_un un;
-#endif
-  } saddr;
+  union u_sockaddr saddr;
   socklen_t len = sizeof saddr;
   ptrdiff_t count;
 
@@ -7978,21 +7977,10 @@ restore_nofile_limit (void)
 }
 
 \f
-DEFUN ("get-external-sockname", Fget_external_sockname, Sget_external_sockname, 0, 0, 0,
-       doc: /* Return the path of an external socket passed to Emacs.
-Otherwise return nil.  */)
-     (void)
-{
-    if (external_sock_name)
-        return make_string(external_sock_name, strlen(external_sock_name));
-    else
-        return Qnil;
-}
-
 /* This is not called "init_process" because that is the name of a
    Mach system call, so it would cause problems on Darwin systems.  */
 void
-init_process_emacs (int sockfd, char *sockname)
+init_process_emacs (int sockfd)
 {
 #ifdef subprocesses
   int i;
@@ -8027,7 +8015,18 @@ init_process_emacs (int sockfd, char *sockname)
 #endif
 
   external_sock_fd = sockfd;
-  external_sock_name = sockname;
+  Lisp_Object sockname = Qnil;
+# if HAVE_GETSOCKNAME
+  if (0 <= sockfd)
+    {
+      union u_sockaddr sa;
+      socklen_t salen = sizeof sa;
+      if (getsockname (sockfd, &sa.sa, &salen) == 0)
+	sockname = conv_sockaddr_to_lisp (&sa.sa, salen);
+    }
+# endif
+  Vinternal__external_sockname = sockname;
+
   max_desc = -1;
   memset (fd_callback_info, 0, sizeof (fd_callback_info));
 
@@ -8220,6 +8219,10 @@ These functions are called in the order of the list, until one of them
 returns non-`nil'.  */);
   Vinterrupt_process_functions = list1 (Qinternal_default_interrupt_process);
 
+  DEFVAR_LISP ("internal--external-sockname", Vinternal__external_sockname,
+	       doc: /* Name of external socket passed to Emacs, or nil if none.  */);
+  Vinternal__external_sockname = Qnil;
+
   DEFSYM (Qinternal_default_interrupt_process,
 	  "internal-default-interrupt-process");
   DEFSYM (Qinterrupt_process_functions, "interrupt-process-functions");
@@ -8322,5 +8325,4 @@ returns non-`nil'.  */);
   defsubr (&Sprocess_inherit_coding_system_flag);
   defsubr (&Slist_system_processes);
   defsubr (&Sprocess_attributes);
-  defsubr (&Sget_external_sockname);
 }
-- 
2.14.3