FvwmMFL: introduce FVWMML_SOCKET_PATH for namespacing

Currently, it's possible to change the listening socket for FvwmMFL via
an environment variable, FVWMMFL_SOCKET. The reasons for this is that it
makes it easier to move the socket path to different locations away from
/tmp, to, say, $FVWM_USERDIR.

However, this single environment variable is reused across different
running fvwm instances which means commands sent via FvwmPrompt or
FvwmCommand end up going to the wrong fvwm instance, as it's a case of
whichever process reads that first, is sent to the DISPLAY that fvwm is
running on.

Therefore, set FVWMMFL_SOCKET_PATH to a base directory, under which,
namespaced directories per DISPLAY for the relevant FvwmMFL sockets can
be created.

Fixes #995

bin/FvwmCommand.in

@@ -9,27 +9,36 @@ import json
 
 # Fvwm3 - socket API and MFL UNIX socket
 # Simple Python 3 FvwmMFL client which purpose is to act as FVWM2 FvwmCommand
-# replacement in FVWM3. It uses FvwmMFL unix domain socket to send FVWM commands
-# at this moment, that is all. It doesn't parse json structures yet, or anything
-# fancy and new from FVWM3 new command processing interface.
+# replacement in FVWM3. It uses FvwmMFL unix domain socket to send FVWM
+# commands at this moment, that is all. It doesn't parse json structures yet,
+# or anything fancy and new from FVWM3 new command processing interface.
 
 def mflclnt(verbose, read_from_stdin, info, fvwm_socket, args):
+    # Check DISPLAY is defined, else bail:
+    if not 'DISPLAY' in os.environ:
+        print("$DISPLAY not set, is xorg/fvwm running?")
+        sys.exit(1)
+    sock_file_name = '/fvwm_mfl_' + str(os.environ.get('DISPLAY')) + '.sock'
+
     # Create a unix domain socket
     sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
 
     # Connect the socket to the "port" where the server is listening
-    # FvwmMFL is setting this in environment. This is done by using
-    # the explicit -f <sockname> flag if it is provided, otherwise
-    # use the environment FVWMMFL_SOCKET if it is defined, then fall
-    # back to TMPDIR/fvwm_mfl.sock if TMPDIR is defined, and last use
-    # the default location /tmp/fvwm_mfl.sock.
+    # FvwmMFL is setting this in environment. This is done by checking
+    # in the following order:
+    #  + Use the explicit -f <sockname> if it is provided.
+    #  + Use FVWMMFL_SOCKET_PATH/fvwm_mfl_DISPLAY.sock if defined.
+    #  + Use TMPDIR/fvwmmfl/fvwm_mfl_DISPLAY.sock if defined.
+    #  + Last use default location /tmp/fvwmmfl/fvwm_mfl_DISPLAY.sock
     if not fvwm_socket:
-        if 'FVWMMFL_SOCKET' in os.environ:
-            fvwm_socket = str(os.environ.get('FVWMMFL_SOCKET'))
+        if 'FVWMMFL_SOCKET_PATH' in os.environ:
+            fvwm_socket = str(os.environ.get('FVWMMFL_SOCKET_PATH'))
+            fvwm_socket += sock_file_name
         elif 'TMPDIR' in os.environ:
-            fvwm_socket = str(os.environ.get('TMPDIR')) + '/fvwm_mfl.sock'
+            fvwm_socket = str(os.environ.get('TMPDIR'))
+            fvwm_socket += '/fvwmmfl' + sock_file_name
         else:
-            fvwm_socket = '/tmp/fvwm_mfl.sock'
+            fvwm_socket = '/tmp/fvwmmfl' + sock_file_name
         def_socket = True
     else:
         def_socket = False
@@ -71,7 +80,7 @@ def mflclnt(verbose, read_from_stdin, info, fvwm_socket, args):
                 sock.sendall(message)
         if verbose:
             print('Sent {!r}'.format(message) + " to FvwmMFL")
-    
+
         amount_received = 0
         data = sock.recv(32768)
         # amount_received += len(data)

doc/FvwmMFL.adoc

@@ -30,9 +30,12 @@ packet has different fields, depending on the type requested.
 
 == COMMUNICATION
 
-The default unix-domain socket for _FvwmMFL_ is _$TMPDIR/fvwm_mfl.sock_,
-although this can be overridden via an environment variable
-_FVWMMFL_SOCKET_.
+The default unix-domain socket for _FvwmMFL_ is
+_$TMPDIR/fvwmmfl/fvwm_mfl_$DISPLAY.sock_.
+
+The path for where _fvwm_mfl-$DISPLAY_.sock is created can be changed by
+setting the _FVWMMFL_SOCKET_PATH_ environment variable.  _FvwmMFL_ will create
+this if it does not exist, and set relevant permissions.
 
 == REGISTERING INTEREST
 
@@ -109,4 +112,4 @@ Outputs:
 
 == AUTHORS
 
-This module first appeared in 2020.
+This module first appeared in 2020.

modules/FvwmMFL/FvwmMFL.c

@@ -47,13 +47,11 @@
 #include <event2/listener.h>
 #include <event2/util.h>
 
-/* This is also configurable via getenv() - see FvwmMFL(1) */
-#define MFL_SOCKET_DEFAULT "fvwm_mfl.sock"
 #define MYNAME "FvwmMFL"
 
 static int debug;
 struct fvwm_msg;
-static char *sock_pathname;
+static char *socket_name, *socket_basepath;
 static char *pid_file;
 
 struct client {
@@ -152,17 +150,16 @@ delete_pid_file(void)
 static void
 set_pid_file(void)
 {
-	char	*fud = getenv("FVWM_USERDIR");
 	char	*dsp = getenv("DISPLAY");
 
-	if (fud == NULL || dsp == NULL) {
+	if (dsp == NULL) {
 		fprintf(stderr,
-		    "FVWM_USERDIR or DISPLAY is not set in the environment\n");
+		    "DISPLAY is not set in the environment\n");
 		exit(1);
 	}
 
 	free(pid_file);
-	xasprintf(&pid_file, "%s/fvwm_mfl_%s.pid", fud, dsp);
+	xasprintf(&pid_file, "%s/fvwm_mfl_%s.pid", socket_basepath, dsp);
 }
 
 static void
@@ -250,7 +247,8 @@ HandleTerminate(int fd, short what, void *arg)
 {
 	fprintf(stderr, "%s: dying...\n", __func__);
 	delete_pid_file();
-	unlink(sock_pathname);
+	unlink(socket_name);
+	rmdir(socket_basepath);
 	fvwmSetTerminate(fd);
 }
 
@@ -787,7 +785,9 @@ fvwm_read(int efd, short ev, void *data)
 	if ((packet = ReadFvwmPacket(efd)) == NULL) {
 		if (debug)
 			fprintf(stderr, "Couldn't read from FVWM - exiting.\n");
-		unlink(sock_pathname);
+		delete_pid_file();
+		unlink(socket_name);
+		rmdir(socket_basepath);
 		exit(0);
 	}
 
@@ -797,40 +797,66 @@ fvwm_read(int efd, short ev, void *data)
 void
 set_socket_pathname(void)
 {
-	char		*mflsock_env, *tmpdir;
+	char		*mflsock_env, *display_env, *tmpdir;
 	const char	*unrolled_path;
+	struct stat	 sb;
 
 	/* Figure out if we are using default MFL socket path or we should
 	 * respect environment variable FVWMMFL_SOCKET for FvwmMFL socket path
 	 */
 
-	mflsock_env = getenv("FVWMMFL_SOCKET");
+	mflsock_env = getenv("FVWMMFL_SOCKET_PATH");
+	display_env = getenv("DISPLAY");
+
 	if (mflsock_env == NULL) {
 		/* Check if TMPDIR is defined.  If so, use that, otherwise
 		 * default to /tmp for the directory name.
 		 */
 		if ((tmpdir = getenv("TMPDIR")) == NULL)
 			tmpdir = "/tmp";
-		xasprintf(&sock_pathname, "%s/%s", tmpdir, MFL_SOCKET_DEFAULT);
-		return;
+		xasprintf(&socket_basepath, "%s/fvwmmfl", tmpdir);
+		goto check_dir;
 	}
 
 	unrolled_path = expand_path(mflsock_env);
 	if (unrolled_path[0] == '/')
-		sock_pathname = fxstrdup(unrolled_path);
+		socket_basepath = fxstrdup(unrolled_path);
 	else {
-		xasprintf(&sock_pathname, "%s/%s", getenv("FVWM_USERDIR"),
+		xasprintf(&socket_basepath, "%s/%s", getenv("FVWM_USERDIR"),
 			unrolled_path);
 	}
 
 	free((void *)unrolled_path);
+
+check_dir:
+	/* Check if what we've been given is a directory */
+	if (lstat(socket_basepath, &sb) != 0 && errno == ENOTDIR) {
+		fprintf(stderr, "Not a directory: socket_basepath: %s\n",
+			socket_basepath);
+		free(socket_basepath);
+		exit(1);
+	}
+
+	/* Try and create the directory.  Only complain if this failed, and
+	 * the directory doesn't already exist.
+	 */
+	if (mkdir(socket_basepath, S_IRWXU) != 0 && errno != EEXIST) {
+		fprintf(stderr, "Coudln't create socket_basepath: %s: %s\n",
+			socket_basepath, strerror(errno));
+		free(socket_basepath);
+		exit(1);
+	}
+
+	xasprintf(&socket_name, "%s/fvwm_mfl_%s.sock", socket_basepath,
+		display_env);
 }
 
 int main(int argc, char **argv)
 {
 	struct event_base     *base;
 	struct evconnlistener *fmd_cfd;
 	struct sockaddr_un    sin;
+	char		     *pe;
 
 	TAILQ_INIT(&clientq);
 
@@ -839,12 +865,13 @@ int main(int argc, char **argv)
 		return (1);
 	}
 
+	set_socket_pathname();
+
 	/* If we're already running... */
 	if (check_pid() == 0)
 		return (1);
 
-	set_socket_pathname();
-	unlink(sock_pathname);
+	unlink(socket_name);
 
 	/* Create new event base */
 	if ((base = event_base_new()) == NULL) {
@@ -855,7 +882,7 @@ int main(int argc, char **argv)
 
 	memset(&sin, 0, sizeof(sin));
 	sin.sun_family = AF_LOCAL;
-	strcpy(sin.sun_path, sock_pathname);
+	strcpy(sin.sun_path, socket_name);
 
 	/* Create a new listener */
 	fmd_cfd = evconnlistener_new_bind(base, accept_conn_cb, NULL,
@@ -867,26 +894,29 @@ int main(int argc, char **argv)
 	}
 	evconnlistener_set_error_cb(fmd_cfd, accept_error_cb);
 
-	chmod(sock_pathname, 0770);
+	chmod(socket_name, 0600);
 
 	/* Setup comms to fvwm3. */
 	fc.fd[0] = fc.m->to_fvwm;
 	fc.fd[1] = fc.m->from_fvwm;
 
-	/* If the user hasn't set FVWMMFL_SOCKET in the environment, then tell
-	 * Fvwm3 to do this.  Note that we have to get Fvwm3 to do this so
-	 * that all other windows can inherit this environment variable.  We
-	 * cannot use flib_putenv() here since this never reaches Fvwm3's
+	/* If the user hasn't set FVWMMFL_SOCKET_PATH in the environment, then
+	 * tell Fvwm3 to do this.  Note that we have to get Fvwm3 to do this
+	 * so that all other windows can inherit this environment variable.
+	 * We cannot use flib_putenv() here since this never reaches Fvwm3's
 	 * environment.
 	 */
-	if (getenv("FVWMMFL_SOCKET") == NULL) {
-		char	*pe;
+	if (getenv("FVWMMFL_SOCKET_PATH") == NULL) {
 
-		xasprintf(&pe, "SetEnv FVWMMFL_SOCKET %s", sock_pathname);
+		xasprintf(&pe, "SetEnv FVWMMFL_SOCKET_PATH %s", socket_basepath);
 		SendText(fc.fd, pe, 0);
 		free(pe);
 	}
 
+	xasprintf(&pe, "SetEnv FVWMMFL_SOCKET %s", socket_name);
+	SendText(fc.fd, pe, 0);
+	free(pe);
+
 	if (evutil_make_socket_nonblocking(fc.fd[0]) < 0)
 		fprintf(stderr, "fvwm to_fvwm socket non-blocking failed");
 	if (evutil_make_socket_nonblocking(fc.fd[1]) < 0)
@@ -900,7 +930,7 @@ int main(int argc, char **argv)
 	SendFinishedStartupNotification(fc.fd);
 
 	event_base_dispatch(base);
-	unlink(sock_pathname);
+	unlink(socket_name);
 
 	return (0);
 }