Move connection-opening to remctl_open()

Take code that selects a port and calls internal_connect() to establish
a connection, and pull it up out of internal_open() and into remctl_open().
In the process, internal_connect() is made non-static, and internal_open()
loses its port argument.

This paves the way for alternatives to remctl_open() in which the caller
provides a socket descriptor or address instead of a hostname and port.
Such interfaces will need internal_open() without internal_connect().

Change-Id: I81d90097a3d422fb9c6a3fe754a5a1f60aac4415
Reviewed-on: https://gerrit.stanford.edu/806
Reviewed-by: Russ Allbery <rra@stanford.edu>
Tested-by: Russ Allbery <rra@stanford.edu>

3 files changed, 32 insertions, 30 deletions

diff --git a/client/api.c b/client/api.c
index 67275b1..2e6ecb1 100644
--- a/client/api.c
+++ b/client/api.c
@@ -313,6 +313,9 @@ int
 remctl_open(struct remctl *r, const char *host, unsigned short port,
             const char *principal)
 {
+    bool port_fallback = false;
+    socket_type fd = INVALID_SOCKET;
+
     if (r->fd != -1) {
         if (r->protocol > 1)
             internal_v2_quit(r);
@@ -330,7 +333,25 @@ remctl_open(struct remctl *r, const char *host, unsigned short port,
     r->host = host;
     r->port = port;
     r->principal = principal;
-    return internal_open(r, host, port, principal);
+
+    /*
+     * If port is 0, default to trying the standard port and then falling back
+     * on the old port.
+     */
+    if (port == 0) {
+        port = REMCTL_PORT;
+        port_fallback = true;
+    }
+
+    /* Make the network connection. */
+    fd = internal_connect(r, host, port);
+    if (fd == INVALID_SOCKET && port_fallback)
+        fd = internal_connect(r, host, REMCTL_PORT_OLD);
+    if (fd == INVALID_SOCKET)
+        return false;
+    r->fd = fd;
+
+    return internal_open(r, host, principal);
 }
 
 
diff --git a/client/internal.h b/client/internal.h
index 9994cb6..b01223d 100644
--- a/client/internal.h
+++ b/client/internal.h
@@ -53,9 +53,11 @@ void internal_token_error(struct remctl *, const char *error, int status,
 /* Wipe and free the output token. */
 void internal_output_wipe(struct remctl_output *);
 
+/* Establish a network connection */
+socket_type internal_connect(struct remctl *, const char *, unsigned short);
+
 /* General connection opening and negotiation function. */
-bool internal_open(struct remctl *, const char *host, unsigned short port,
-                   const char *principal);
+bool internal_open(struct remctl *, const char *host, const char *principal);
 
 /* Send a protocol v1 command. */
 bool internal_v1_commandv(struct remctl *, const struct iovec *command,
diff --git a/client/open.c b/client/open.c
index 14382c1..3d9bc67 100644
--- a/client/open.c
+++ b/client/open.c
@@ -33,7 +33,7 @@
  * network connection.  Returns the file descriptor if successful or
  * INVALID_SOCKET on failure.
  */
-static socket_type
+socket_type
 internal_connect(struct remctl *r, const char *host, unsigned short port)
 {
     struct addrinfo hints, *ai;
@@ -126,12 +126,9 @@ internal_import_name(struct remctl *r, const char *host,
  * failure.  On failure, sets the error message appropriately.
  */
 bool
-internal_open(struct remctl *r, const char *host, unsigned short port,
-              const char *principal)
+internal_open(struct remctl *r, const char *host, const char *principal)
 {
     int status, flags;
-    bool port_fallback = false;
-    socket_type fd = INVALID_SOCKET;
     gss_buffer_desc send_tok, recv_tok, *token_ptr;
     gss_buffer_desc empty_token = { 0, (void *) "" };
     gss_name_t name = GSS_C_NO_NAME;
@@ -143,23 +140,6 @@ internal_open(struct remctl *r, const char *host, unsigned short port,
     static const OM_uint32 req_gss_flags
         = (GSS_C_MUTUAL_FLAG | GSS_C_CONF_FLAG | GSS_C_INTEG_FLAG);
 
-    /*
-     * If port is 0, default to trying the standard port and then falling back
-     * on the old port.
-     */
-    if (port == 0) {
-        port = REMCTL_PORT;
-        port_fallback = true;
-    }
-
-    /* Make the network connection. */
-    fd = internal_connect(r, host, port);
-    if (fd == INVALID_SOCKET && port_fallback)
-        fd = internal_connect(r, host, REMCTL_PORT_OLD);
-    if (fd == INVALID_SOCKET)
-        goto fail;
-    r->fd = fd;
-
     /* Import the name. */
     if (!internal_import_name(r, host, principal, &name))
         goto fail;
@@ -173,7 +153,7 @@ internal_open(struct remctl *r, const char *host, unsigned short port,
         r->protocol = 2;
 
     /* Send the initial negotiation token. */
-    status = token_send(fd, TOKEN_NOOP | TOKEN_CONTEXT_NEXT | TOKEN_PROTOCOL,
+    status = token_send(r->fd, TOKEN_NOOP | TOKEN_CONTEXT_NEXT | TOKEN_PROTOCOL,
                         &empty_token, r->timeout);
     if (status != TOKEN_OK) {
         internal_token_error(r, "sending initial token", status, 0, 0);
@@ -211,7 +191,7 @@ internal_open(struct remctl *r, const char *host, unsigned short port,
             flags = TOKEN_CONTEXT;
             if (r->protocol > 1)
                 flags |= TOKEN_PROTOCOL;
-            status = token_send(fd, flags, &send_tok, r->timeout);
+            status = token_send(r->fd, flags, &send_tok, r->timeout);
             if (status != TOKEN_OK) {
                 internal_token_error(r, "sending token", status, major, minor);
                 gss_release_buffer(&minor, &send_tok);
@@ -229,7 +209,7 @@ internal_open(struct remctl *r, const char *host, unsigned short port,
 
         /* If we're still expecting more, retrieve it. */
         if (major == GSS_S_CONTINUE_NEEDED) {
-            status = token_recv(fd, &flags, &recv_tok, TOKEN_MAX_LENGTH,
+            status = token_recv(r->fd, &flags, &recv_tok, TOKEN_MAX_LENGTH,
                                 r->timeout);
             if (status != TOKEN_OK) {
                 internal_token_error(r, "receiving token", status, major,
@@ -261,8 +241,7 @@ internal_open(struct remctl *r, const char *host, unsigned short port,
     return true;
 
 fail:
-    if (fd != INVALID_SOCKET)
-        socket_close(fd);
+    socket_close(r->fd);
     r->fd = INVALID_SOCKET;
     if (name != GSS_C_NO_NAME)
         gss_release_name(&minor, &name);