Hi Łukasz,

So I was taking a closer look at the libxtrans patch today,
and I noticed that it does not build. It adds a check for
trans->flags&TRANS_RECEIVED inside the main loop in
MakeAllCOTSServerListeners. But TRANS_RECEIVED does not get
defined anywhere.

I've added a define for it to Xtransint.h, which fixes the
build issue. But other then fixing the BuildIssue this
makes little sense, since the flag is never being set.

So an alternative approach to fixing the build issue, would
to just remove the addition of the check from your patch.

Which has left me wondering why did you add this check in
the first place? What was it supposed to do, and if we
drop the check, do we need some other code to achieve
the same result?

While looking into this, I've polished the patch a but
up, addressing some of the review comments from its
earlier posting, and improving error reporting. I've
attached my cleaned-up version.

Regards,

Hans
>From 30f7d7900f8b74e3bd4f6ecae1a561ee5cf72a5a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Stelmach?= <[email protected]>
Date: Mon, 25 Nov 2013 11:11:54 +0100
Subject: [PATCH] Enable systemd socket activation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Receive file descriptors of open sockets from systemd instead of
creating them.

Signed-off-by: Łukasz Stelmach <[email protected]>
Cc: Kyungmin Park <[email protected]>
Cc: MyungJoo Ham <[email protected]>
Cc: Piort Bereza <[email protected]>
Cc: Karol Lewandowski <[email protected]>
Cc: Lennart Poettering <[email protected]>
Cc: Zbigniew Jędrzejewski-Szmek <[email protected]>
Cc: Peter Hutterer <[email protected]>
Cc: walter harms <[email protected]>
Cc: Alan Coopersmith <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
 Xtrans.c    | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 Xtransint.h |  1 +
 2 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/Xtrans.c b/Xtrans.c
index 735d7b8..8b67194 100644
--- a/Xtrans.c
+++ b/Xtrans.c
@@ -48,6 +48,9 @@ from The Open Group.
  */
 
 #include <ctype.h>
+#ifdef HAVE_SYSTEMD
+#include <systemd/sd-daemon.h>
+#endif
 
 /*
  * The transport table contains a definition for every transport (protocol)
@@ -1023,6 +1026,79 @@ complete_network_count (void)
 }
 
 
+static int
+receive_listening_fds(char* port, XtransConnInfo* temp_ciptrs, int* count_ret)
+
+{
+#ifdef HAVE_SYSTEMD
+    XtransConnInfo	ciptr;
+    int  systemd_listen_fds, i;
+
+    systemd_listen_fds = sd_listen_fds(1);
+    if (systemd_listen_fds < 0)
+    {
+        prmsg (1, "receive_listening_fds:"
+               "sd_listen_fds failed, error %d\n", systemd_listen_fds);
+	return -1;
+    }
+
+    for (i = 0; i < systemd_listen_fds && *count_ret < NUMTRANS; i++)
+    {
+        struct sockaddr_storage a;
+        int ti;
+        const char* tn;
+        socklen_t al;
+
+        al = sizeof(a);
+        if (getsockname(i + SD_LISTEN_FDS_START, (struct sockaddr*)&a, &al) < 0) {
+            prmsg (1, "receive_listening_fds:"
+                   "getsockname failed, error %d\n", errno);
+            return -1;
+        }
+
+        switch (a.ss_family)
+        {
+        case AF_UNIX:
+            ti = TRANS_SOCKET_UNIX_INDEX;
+            if (*((struct sockaddr_un*)&a)->sun_path == '\0' &&
+                al > sizeof(sa_family_t))
+                tn = "local";
+            else
+                tn = "unix";
+            break;
+        case AF_INET:
+            ti = TRANS_SOCKET_INET_INDEX;
+            tn = "inet";
+            break;
+#if defined(IPv6) && defined(AF_INET6)
+        case AF_INET6:
+            ti = TRANS_SOCKET_INET6_INDEX;
+            tn = "inet6";
+            break;
+#endif /* IPv6 */
+        default:
+            prmsg (1, "receive_listening_fds:"
+                   "Got unknown socket address family\n");
+            return -1;
+        }
+
+        ciptr = TRANS(ReopenCOTSServer)(ti, i + SD_LISTEN_FDS_START, port);
+        if (!ciptr)
+        {
+            prmsg (1, "receive_listening_fds:"
+                   "Got NULL while trying to reopen socket received from systemd.\n");
+            return -1;
+        }
+
+        prmsg (5, "receive_listening_fds: received listener for %s, %d\n",
+               tn, ciptr->fd);
+        temp_ciptrs[(*count_ret)++] = ciptr;
+        TRANS(Received)(tn);
+    }
+#endif /* HAVE_SYSTEMD */
+    return 0;
+}
+
 #ifdef XQUARTZ_EXPORTS_LAUNCHD_FD
 extern int xquartz_launchd_fd;
 #endif
@@ -1055,12 +1131,16 @@ TRANS(MakeAllCOTSServerListeners) (char *port, int *partial, int *count_ret,
     }
 #endif
 
+    if (receive_listening_fds(port, temp_ciptrs, count_ret) < 0)
+	return -1;
+
     for (i = 0; i < NUMTRANS; i++)
     {
 	Xtransport *trans = Xtransports[i].transport;
 	unsigned int flags = 0;
 
-	if (trans->flags&TRANS_ALIAS || trans->flags&TRANS_NOLISTEN)
+	if (trans->flags&TRANS_ALIAS || trans->flags&TRANS_NOLISTEN ||
+	    trans->flags&TRANS_RECEIVED)
 	    continue;
 
 	snprintf(buffer, sizeof(buffer), "%s/:%s",
diff --git a/Xtransint.h b/Xtransint.h
index 1f32f0c..611150a 100644
--- a/Xtransint.h
+++ b/Xtransint.h
@@ -335,6 +335,7 @@ typedef struct _Xtransport_table {
 #define TRANS_NOUNLINK	(1<<4)	/* Don't unlink transport endpoints */
 #define TRANS_ABSTRACT	(1<<5)	/* Use abstract sockets if available */
 #define TRANS_NOXAUTH	(1<<6)	/* Don't verify authentication (because it's secure some other way at the OS layer) */
+#define TRANS_RECEIVED	(1<<7)	/* This socket was received from systemd */
 
 /* Flags to preserve when setting others */
 #define TRANS_KEEPFLAGS	(TRANS_NOUNLINK|TRANS_ABSTRACT)
-- 
1.8.4.2

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to