Clean up Unix Pageant's setup and teardown.

I've moved the listening socket setup back to before the lifetime
preparations, so in particular we find out that we couldn't bind to
the socket _before_ we fork. The only part that really needed to come
after lifetime setup was the logging setup, so that's now a separate
function called later.

Also, the random exit(0)s in silly places like x11_closing have turned
into setting a time_to_die flag, so that all clean exits funnel back
to the end of main() which at least tries to tidy up a bit afterwards.

(Finally, fixed a small bug in testing the return value of waitpid(),
which only showed up once we didn't exit(0) after the first wait.
Ahem.)

diff --git a/pageant.c b/pageant.c
index 495124a57d25ae4c1879374c9694ee45cdc85372..f6383a3c9403a1ba3ac08375a81461225d45997b 100644 (file)
--- a/pageant.c
+++ b/pageant.c
@@ -1147,8 +1147,7 @@ static int pageant_listen_accepting(Plug plug,
     return 0;
 }
 
-struct pageant_listen_state *pageant_listener_new(void *logctx,
-                                                  pageant_logfn_t logfn)
+struct pageant_listen_state *pageant_listener_new(void)
 {
     static const struct plug_function_table listener_fn_table = {
         NULL, /* no log function, because that's for outgoing connections */
@@ -1160,8 +1159,8 @@ struct pageant_listen_state *pageant_listener_new(void *logctx,
 
     struct pageant_listen_state *pl = snew(struct pageant_listen_state);
     pl->fn = &listener_fn_table;
-    pl->logctx = logctx;
-    pl->logfn = logfn;
+    pl->logctx = NULL;
+    pl->logfn = NULL;
     pl->listensock = NULL;
     return pl;
 }
@@ -1171,6 +1170,13 @@ void pageant_listener_got_socket(struct pageant_listen_state *pl, Socket sock)
     pl->listensock = sock;
 }
 
+void pageant_listener_set_logfn(struct pageant_listen_state *pl,
+                                void *logctx, pageant_logfn_t logfn)
+{
+    pl->logctx = logctx;
+    pl->logfn = logfn;
+}
+
 void pageant_listener_free(struct pageant_listen_state *pl)
 {
     if (pl->listensock)

diff --git a/pageant.h b/pageant.h
index 4d53cbec21a8905a67af2fc892f43d9e6e00e1d2..06dd033a8010fd6cd8d412afa6cd388c8db46657 100644 (file)
--- a/pageant.h
+++ b/pageant.h
@@ -76,10 +76,12 @@ void keylist_update(void);
  * protocol. Call pageant_listener_new() to set up a state; then
  * create a socket using the returned pointer as a Plug; then call
  * pageant_listener_got_socket() to give the listening state its own
- * socket pointer.
+ * socket pointer. Also, provide a logging function later if you want
+ * to.
  */
 struct pageant_listen_state;
-struct pageant_listen_state *pageant_listener_new(void *logctx,
-                                                  pageant_logfn_t logfn);
+struct pageant_listen_state *pageant_listener_new(void);
 void pageant_listener_got_socket(struct pageant_listen_state *pl, Socket sock);
+void pageant_listener_set_logfn(struct pageant_listen_state *pl,
+                                void *logctx, pageant_logfn_t logfn);
 void pageant_listener_free(struct pageant_listen_state *pl);

diff --git a/unix/uxpgnt.c b/unix/uxpgnt.c
index 5c3772c80215bd11fe47e104cdb11e1c2775445e..c0b63c7804b931b3af0dd106df9cc0a49f89cacc 100644 (file)
--- a/unix/uxpgnt.c
+++ b/unix/uxpgnt.c
@@ -130,28 +130,44 @@ void keylist_update(void)
 
 const char *const appname = "Pageant";
 
-Conf *conf;
-
 char *platform_get_x_display(void) {
     return dupstr(getenv("DISPLAY"));
 }
+
+static int time_to_die = FALSE;
+
+/* Stub functions to permit linking against x11fwd.c. These never get
+ * used, because in LIFE_X11 mode we connect to the X server using a
+ * straightforward Socket and don't try to create an ersatz SSH
+ * forwarding too. */
 int sshfwd_write(struct ssh_channel *c, char *data, int len) { return 0; }
-void sshfwd_write_eof(struct ssh_channel *c) { /* FIXME: notify main loop instead */ exit(0); }
-void sshfwd_unclean_close(struct ssh_channel *c, const char *err) { /* FIXME: notify main loop instead */ exit(1); }
+void sshfwd_write_eof(struct ssh_channel *c) { }
+void sshfwd_unclean_close(struct ssh_channel *c, const char *err) { }
 void sshfwd_unthrottle(struct ssh_channel *c, int bufsize) {}
-Conf *sshfwd_get_conf(struct ssh_channel *c) { return conf; }
+Conf *sshfwd_get_conf(struct ssh_channel *c) { return NULL; }
 void sshfwd_x11_sharing_handover(struct ssh_channel *c,
                                  void *share_cs, void *share_chan,
                                  const char *peer_addr, int peer_port,
                                  int endian, int protomajor, int protominor,
                                  const void *initial_data, int initial_len) {}
 void sshfwd_x11_is_local(struct ssh_channel *c) {}
+
+/*
+ * These functions are part of the plug for our connection to the X
+ * display, so they do get called. They needn't actually do anything,
+ * except that x11_closing has to signal back to the main loop that
+ * it's time to terminate.
+ */
 static void x11_log(Plug p, int type, SockAddr addr, int port,
                    const char *error_msg, int error_code) {}
-static int x11_closing(Plug plug, const char *error_msg, int error_code,
-                      int calling_back) { /* FIXME: notify main loop instead */ exit(0); }
-static int x11_receive(Plug plug, int urgent, char *data, int len) { return 0; }
+static int x11_receive(Plug plug, int urgent, char *data, int len) {return 0;}
 static void x11_sent(Plug plug, int bufsize) {}
+static int x11_closing(Plug plug, const char *error_msg, int error_code,
+                      int calling_back)
+{
+    time_to_die = TRUE;
+    return 1;
+}
 struct X11Connection {
     const struct plug_function_table *fn;
 };
@@ -224,6 +240,7 @@ int main(int argc, char **argv)
     int doing_opts = TRUE;
     char **exec_args = NULL;
     int termination_pid = -1;
+    Conf *conf;
 
     fdlist = NULL;
     fdcount = fdsize = 0;
@@ -297,6 +314,14 @@ int main(int argc, char **argv)
         exit(1);
     }
     socketname = dupprintf("%s/pageant.%d", socketdir, (int)getpid());
+    pageant_init();
+    pl = pageant_listener_new();
+    sock = new_unix_listener(unix_sock_addr(socketname), (Plug)pl);
+    if ((err = sk_socket_error(sock)) != NULL) {
+        fprintf(stderr, "pageant: %s: %s\n", socketname, err);
+        exit(1);
+    }
+    pageant_listener_got_socket(pl, sock);
 
     conf = conf_new();
     conf_set_int(conf, CONF_proxy_type, PROXY_NONE);
@@ -380,18 +405,15 @@ int main(int argc, char **argv)
         }
     }
 
-    pageant_init();
-    pl = pageant_listener_new(NULL, pageant_logfp ? pageant_log : NULL);
-    sock = new_unix_listener(unix_sock_addr(socketname), (Plug)pl);
-    if ((err = sk_socket_error(sock)) != NULL) {
-        fprintf(stderr, "pageant: %s: %s\n", socketname, err);
-        exit(1);
-    }
-    pageant_listener_got_socket(pl, sock);
+    /*
+     * Now we've decided on our logging arrangements, pass them on to
+     * pageant.c.
+     */
+    pageant_listener_set_logfn(pl, NULL, pageant_logfp ? pageant_log : NULL);
 
     now = GETTICKCOUNT();
 
-    while (1) {
+    while (!time_to_die) {
        fd_set rset, wset, xset;
        int maxfd;
        int rwx;
@@ -493,15 +515,24 @@ int main(int argc, char **argv)
                 int status;
                 pid_t pid;
                 pid = waitpid(-1, &status, WNOHANG);
-                if (pid == 0)
+                if (pid <= 0)
                     break;
                 if (pid == termination_pid)
-                    exit(0);
+                    time_to_die = TRUE;
             }
         }
 
         run_toplevel_callbacks();
     }
 
+    /*
+     * When we come here, we're terminating, and should clean up our
+     * Unix socket file if possible.
+     */
+    if (unlink(socketname) < 0) {
+        fprintf(stderr, "pageant: %s: %s\n", socketname, strerror(errno));
+        exit(1);
+    }
+
     return 0;
 }