New API for plug_closing() with a custom type enum.

Passing an operating-system-specific error code to plug_closing(),
such as errno or GetLastError(), was always a bit weird, given that it
generally had to be handled by cross-platform receiving code in
backends. I had the platform.h implementations #define any error
values that the cross-platform code would have to handle specially,
but that's still not a great system, because it also doesn't leave
freedom to invent error representations of my own that don't
correspond to any OS code. (For example, the ones I just removed from
proxy.h.)

So now, the OS error code is gone from the plug_closing API, and in
its place is a custom enumeration of closure types: normal, error, and
the special case BROKEN_PIPE which is the only OS error code we have
so far needed to handle specially. (All others just mean 'abandon the
connection and print the textual message'.)

Having already centralised the handling of OS error codes in the
previous commit, we've now got a convenient place to add any further
type codes for errors needing special handling: each of Unix
plug_closing_errno(), Windows plug_closing_system_error(), and Windows
plug_closing_winsock_error() can easily grow extra special cases if
need be, and each one will only have to live in one place.

network.h

@@ -51,6 +51,12 @@ typedef enum PlugLogType {
     PLUGLOG_PROXY_MSG,
 } PlugLogType;
 
+typedef enum PlugCloseType {
+    PLUGCLOSE_NORMAL,
+    PLUGCLOSE_ERROR,
+    PLUGCLOSE_BROKEN_PIPE,
+} PlugCloseType;
+
 struct PlugVtable {
     /*
      * Passes the client progress reports on the process of setting
@@ -88,18 +94,26 @@ struct PlugVtable {
                 const char *error_msg, int error_code);
 
     /*
-     * Notifies the Plug that the socket is closing.
+     * Notifies the Plug that the socket is closing, and something
+     * about why.
      *
-     * For a normal non-error close, error_msg is NULL. If the socket
+     *  - PLUGCLOSE_NORMAL means an ordinary non-error closure. In
-     * has encountered an error, error_msg will contain a string
+     *    this case, error_msg should be ignored (and hopefully
-     * (ownership not transferred), and error_code will contain the OS
+     *    callers will have passed NULL).
-     * error code, if available.
      *
-     * OS error codes will vary between platforms, of course, but
+     *  - PLUGCLOSE_ERROR indicates that an OS error occurred, and
-     * platform.h should define any that we need to distinguish here,
+     *    'error_msg' contains a string describing it, for use in
-     * in particular BROKEN_PIPE_ERROR_CODE.
+     *    diagnostics. (Ownership of the string is not transferred.)
+     *    This error class covers anything other than the special
+     *    case below:
+     *
+     *  - PLUGCLOSE_BROKEN_PIPE behaves like PLUGCLOSE_ERROR (in
+     *    particular, there's still an error message provided), but
+     *    distinguishes the particular error condition signalled by
+     *    EPIPE / ERROR_BROKEN_PIPE, which ssh/sharing.c needs to
+     *    recognise and handle specially in one situation.
      */
-    void (*closing)(Plug *p, const char *error_msg, int error_code);
+    void (*closing)(Plug *p, PlugCloseType type, const char *error_msg);
 
     /*
      * Provides incoming socket data to the Plug. Three cases:
@@ -222,12 +236,12 @@ static inline void sk_write_eof(Socket *s)
 static inline void plug_log(
     Plug *p, int type, SockAddr *addr, int port, const char *msg, int code)
 { p->vt->log(p, type, addr, port, msg, code); }
-static inline void plug_closing(Plug *p, const char *msg, int code)
+static inline void plug_closing(Plug *p, PlugCloseType type, const char *msg)
-{ p->vt->closing(p, msg, code); }
+{ p->vt->closing(p, type, msg); }
 static inline void plug_closing_normal(Plug *p)
-{ p->vt->closing(p, NULL, 0); }
+{ p->vt->closing(p, PLUGCLOSE_NORMAL, NULL); }
 static inline void plug_closing_error(Plug *p, const char *msg)
-{ p->vt->closing(p, msg, 0); }
+{ p->vt->closing(p, PLUGCLOSE_ERROR, msg); }
 static inline void plug_receive(Plug *p, int urg, const char *data, size_t len)
 { p->vt->receive(p, urg, data, len); }
 static inline void plug_sent (Plug *p, size_t bufsize)
@@ -352,7 +366,7 @@ extern Plug *const nullplug;
  */
 void nullplug_log(Plug *plug, PlugLogType type, SockAddr *addr,
                   int port, const char *err_msg, int err_code);
-void nullplug_closing(Plug *plug, const char *error_msg, int error_code);
+void nullplug_closing(Plug *plug, PlugCloseType type, const char *error_msg);
 void nullplug_receive(Plug *plug, int urgent, const char *data, size_t len);
 void nullplug_sent(Plug *plug, size_t bufsize);
 

nullplug.c

@@ -12,7 +12,7 @@ void nullplug_log(Plug *plug, PlugLogType type, SockAddr *addr,
 {
 }
 
-void nullplug_closing(Plug *plug, const char *error_msg, int error_code)
+void nullplug_closing(Plug *plug, PlugCloseType type, const char *error_msg)
 {
 }
 

otherbackends/raw.c

@@ -65,11 +65,11 @@ static void raw_check_close(Raw *raw)
     }
 }
 
-static void raw_closing(Plug *plug, const char *error_msg, int error_code)
+static void raw_closing(Plug *plug, PlugCloseType type, const char *error_msg)
 {
     Raw *raw = container_of(plug, Raw, plug);
 
-    if (error_msg) {
+    if (type != PLUGCLOSE_NORMAL) {
         /* A socket error has occurred. */
         if (raw->s) {
             sk_close(raw->s);

otherbackends/rlogin.c

@@ -85,7 +85,8 @@ static void rlogin_log(Plug *plug, PlugLogType type, SockAddr *addr, int port,
     }
 }
 
-static void rlogin_closing(Plug *plug, const char *error_msg, int error_code)
+static void rlogin_closing(Plug *plug, PlugCloseType type,
+                           const char *error_msg)
 {
     Rlogin *rlogin = container_of(plug, Rlogin, plug);
 
@@ -103,11 +104,12 @@ static void rlogin_closing(Plug *plug, const char *error_msg, int error_code)
         seat_notify_remote_exit(rlogin->seat);
         seat_notify_remote_disconnect(rlogin->seat);
     }
-    if (error_msg) {
+    if (type != PLUGCLOSE_NORMAL) {
         /* A socket error has occurred. */
         logevent(rlogin->logctx, error_msg);
         seat_connection_fatal(rlogin->seat, "%s", error_msg);
-    }                                  /* Otherwise, the remote side closed the connection normally. */
+    }
+    /* Otherwise, the remote side closed the connection normally. */
 }
 
 static void rlogin_receive(

otherbackends/supdup.c

@@ -573,7 +573,8 @@ static void supdup_log(Plug *plug, PlugLogType type, SockAddr *addr, int port,
     }
 }
 
-static void supdup_closing(Plug *plug, const char *error_msg, int error_code)
+static void supdup_closing(Plug *plug, PlugCloseType type,
+                           const char *error_msg)
 {
     Supdup *supdup = container_of(plug, Supdup, plug);
 
@@ -591,7 +592,7 @@ static void supdup_closing(Plug *plug, const char *error_msg, int error_code)
         seat_notify_remote_exit(supdup->seat);
         seat_notify_remote_disconnect(supdup->seat);
     }
-    if (error_msg) {
+    if (type != PLUGCLOSE_NORMAL) {
         logevent(supdup->logctx, error_msg);
         seat_connection_fatal(supdup->seat, "%s", error_msg);
     }

otherbackends/telnet.c

@@ -635,7 +635,8 @@ static void telnet_log(Plug *plug, PlugLogType type, SockAddr *addr, int port,
     }
 }
 
-static void telnet_closing(Plug *plug, const char *error_msg, int error_code)
+static void telnet_closing(Plug *plug, PlugCloseType type,
+                           const char *error_msg)
 {
     Telnet *telnet = container_of(plug, Telnet, plug);
 
@@ -653,7 +654,7 @@ static void telnet_closing(Plug *plug, const char *error_msg, int error_code)
         seat_notify_remote_exit(telnet->seat);
         seat_notify_remote_disconnect(telnet->seat);
     }
-    if (error_msg) {
+    if (type != PLUGCLOSE_NORMAL) {
         logevent(telnet->logctx, error_msg);
         seat_connection_fatal(telnet->seat, "%s", error_msg);
     }

pageant.c

@@ -1463,12 +1463,12 @@ struct pageant_conn_state {
     Plug plug;
 };
 
-static void pageant_conn_closing(Plug *plug, const char *error_msg,
+static void pageant_conn_closing(Plug *plug, PlugCloseType type,
-                                 int error_code)
+                                 const char *error_msg)
 {
     struct pageant_conn_state *pc = container_of(
         plug, struct pageant_conn_state, plug);
-    if (error_msg)
+    if (type != PLUGCLOSE_NORMAL)
         pageant_listener_client_log(pc->plc, "c#%"SIZEu": error: %s",
                                     pc->conn_index, error_msg);
     else
@@ -1610,12 +1610,12 @@ struct pageant_listen_state {
     Plug plug;
 };
 
-static void pageant_listen_closing(Plug *plug, const char *error_msg,
+static void pageant_listen_closing(Plug *plug, PlugCloseType type,
-                                   int error_code)
+                                   const char *error_msg)
 {
     struct pageant_listen_state *pl = container_of(
         plug, struct pageant_listen_state, plug);
-    if (error_msg)
+    if (type != PLUGCLOSE_NORMAL)
         pageant_listener_client_log(pl->plc, "listening socket: error: %s",
                                     error_msg);
     sk_close(pl->listensock);

proxy/proxy.c

@@ -181,16 +181,17 @@ static void plug_proxy_log(Plug *plug, PlugLogType type, SockAddr *addr,
     plug_log(ps->plug, type, addr, port, error_msg, error_code);
 }
 
-static void plug_proxy_closing(Plug *p, const char *error_msg, int error_code)
+static void plug_proxy_closing(Plug *p, PlugCloseType type,
+                               const char *error_msg)
 {
     ProxySocket *ps = container_of(p, ProxySocket, plugimpl);
 
     if (ps->state != PROXY_STATE_ACTIVE) {
+        ps->closing_type = type;
         ps->closing_error_msg = error_msg;
-        ps->closing_error_code = error_code;
         ps->negotiate(ps, PROXY_CHANGE_CLOSING);
     } else {
-        plug_closing(ps->plug, error_msg, error_code);
+        plug_closing(ps->plug, type, error_msg);
     }
 }
 
@@ -615,7 +616,7 @@ int proxy_http_negotiate (ProxySocket *p, int change)
          * a socket close, then some error must have occurred. we'll
          * just pass those errors up to the backend.
          */
-        plug_closing(p->plug, p->closing_error_msg, p->closing_error_code);
+        plug_closing(p->plug, p->closing_type, p->closing_error_msg);
         return 0; /* ignored */
     }
 
@@ -803,7 +804,7 @@ int proxy_socks4_negotiate (ProxySocket *p, int change)
          * a socket close, then some error must have occurred. we'll
          * just pass those errors up to the backend.
          */
-        plug_closing(p->plug, p->closing_error_msg, p->closing_error_code);
+        plug_closing(p->plug, p->closing_type, p->closing_error_msg);
         return 0; /* ignored */
     }
 
@@ -945,7 +946,7 @@ int proxy_socks5_negotiate (ProxySocket *p, int change)
          * a socket close, then some error must have occurred. we'll
          * just pass those errors up to the backend.
          */
-        plug_closing(p->plug, p->closing_error_msg, p->closing_error_code);
+        plug_closing(p->plug, p->closing_type, p->closing_error_msg);
         return 0; /* ignored */
     }
 
@@ -1467,7 +1468,7 @@ int proxy_telnet_negotiate (ProxySocket *p, int change)
          * a socket close, then some error must have occurred. we'll
          * just pass those errors up to the backend.
          */
-        plug_closing(p->plug, p->closing_error_msg, p->closing_error_code);
+        plug_closing(p->plug, p->closing_type, p->closing_error_msg);
         return 0; /* ignored */
     }
 

proxy/proxy.h

@@ -58,8 +58,8 @@ struct ProxySocket {
      */
 
     /* closing */
+    PlugCloseType closing_type;
     const char *closing_error_msg;
-    int closing_error_code;
 
     /* receive */
     bool receive_urgent;

psocks.c

@@ -94,8 +94,7 @@ static const SshChannelVtable psocks_scvt = {
 
 static void psocks_plug_log(Plug *p, PlugLogType type, SockAddr *addr,
                             int port, const char *error_msg, int error_code);
-static void psocks_plug_closing(Plug *p, const char *error_msg,
+static void psocks_plug_closing(Plug *p, PlugCloseType, const char *error_msg);
-                                int error_code);
 static void psocks_plug_receive(Plug *p, int urgent,
                                 const char *data, size_t len);
 static void psocks_plug_sent(Plug *p, size_t bufsize);
@@ -354,8 +353,8 @@ static void psocks_plug_log(Plug *plug, PlugLogType type, SockAddr *addr,
     };
 }
 
-static void psocks_plug_closing(Plug *plug, const char *error_msg,
+static void psocks_plug_closing(Plug *plug, PlugCloseType type,
-                                int error_code)
+                                const char *error_msg)
 {
     psocks_connection *conn = container_of(plug, psocks_connection, plug);
     if (conn->connecting) {

ssh/portfwd.c

@@ -109,12 +109,12 @@ static void pfl_log(Plug *plug, PlugLogType type, SockAddr *addr, int port,
 
 static void pfd_close(struct PortForwarding *pf);
 
-static void pfd_closing(Plug *plug, const char *error_msg, int error_code)
+static void pfd_closing(Plug *plug, PlugCloseType type, const char *error_msg)
 {
     struct PortForwarding *pf =
         container_of(plug, struct PortForwarding, plug);
 
-    if (error_msg) {
+    if (type != PLUGCLOSE_NORMAL) {
         /*
          * Socket error. Slam the connection instantly shut.
          */
@@ -141,7 +141,7 @@ static void pfd_closing(Plug *plug, const char *error_msg, int error_code)
 
 static void pfl_terminate(struct PortListener *pl);
 
-static void pfl_closing(Plug *plug, const char *error_msg, int error_code)
+static void pfl_closing(Plug *plug, PlugCloseType type, const char *error_msg)
 {
     struct PortListener *pl = (struct PortListener *) plug;
     pfl_terminate(pl);

ssh/server.c

@@ -140,10 +140,11 @@ static void server_socket_log(Plug *plug, PlugLogType type, SockAddr *addr,
     /* FIXME */
 }
 
-static void server_closing(Plug *plug, const char *error_msg, int error_code)
+static void server_closing(Plug *plug, PlugCloseType type,
+                           const char *error_msg)
 {
     server *srv = container_of(plug, server, plug);
-    if (error_msg) {
+    if (type != PLUGCLOSE_NORMAL) {
         ssh_remote_error(&srv->ssh, "%s", error_msg);
     } else if (srv->bpp) {
         srv->bpp->input_eof = true;

ssh/sesschan.c

@@ -367,7 +367,7 @@ bool sesschan_run_subsystem(Channel *chan, ptrlen subsys)
 static void fwd_log(Plug *plug, PlugLogType type, SockAddr *addr, int port,
                     const char *error_msg, int error_code)
 { /* don't expect any weirdnesses from a listening socket */ }
-static void fwd_closing(Plug *plug, const char *error_msg, int error_code)
+static void fwd_closing(Plug *plug, PlugCloseType type, const char *error_msg)
 { /* not here, either */ }
 
 static int xfwd_accepting(Plug *p, accept_fn_t constructor, accept_ctx_t ctx)

ssh/sharing.c

@@ -937,13 +937,12 @@ static void share_disconnect(struct ssh_sharing_connstate *cs,
     share_begin_cleanup(cs);
 }
 
-static void share_closing(Plug *plug, const char *error_msg, int error_code)
+static void share_closing(Plug *plug, PlugCloseType type,
+                          const char *error_msg)
 {
     struct ssh_sharing_connstate *cs = container_of(
         plug, struct ssh_sharing_connstate, plug);
 
-    if (error_msg) {
-#ifdef BROKEN_PIPE_ERROR_CODE
     /*
      * Most of the time, we log what went wrong when a downstream
      * disappears with a socket error. One exception, though, is
@@ -953,10 +952,9 @@ static void share_closing(Plug *plug, const char *error_msg, int error_code)
      * closing it again without bothering to read our version string).
      * So that one case is not treated as a log-worthy error.
      */
-        if (error_code == BROKEN_PIPE_ERROR_CODE && !cs->got_verstring)
+    if (type == PLUGCLOSE_BROKEN_PIPE && !cs->got_verstring) {
         /* do nothing */;
-        else
+    } else if (type != PLUGCLOSE_NORMAL) {
-#endif
         log_downstream(cs, "Socket error: %s", error_msg);
     }
     share_begin_cleanup(cs);
@@ -1844,12 +1842,12 @@ static void share_sent(Plug *plug, size_t bufsize)
      */
 }
 
-static void share_listen_closing(Plug *plug, const char *error_msg,
+static void share_listen_closing(Plug *plug, PlugCloseType type,
-                                 int error_code)
+                                 const char *error_msg)
 {
     ssh_sharing_state *sharestate =
         container_of(plug, ssh_sharing_state, plug);
-    if (error_msg)
+    if (type != PLUGCLOSE_NORMAL)
         log_general(sharestate, "listening socket: %s", error_msg);
     sk_close(sharestate->listensock);
     sharestate->listensock = NULL;

ssh/ssh.c

@@ -603,10 +603,10 @@ static void ssh_socket_log(Plug *plug, PlugLogType type, SockAddr *addr,
                            ssh->session_started);
 }
 
-static void ssh_closing(Plug *plug, const char *error_msg, int error_code)
+static void ssh_closing(Plug *plug, PlugCloseType type, const char *error_msg)
 {
     Ssh *ssh = container_of(plug, Ssh, plug);
-    if (error_msg) {
+    if (type != PLUGCLOSE_NORMAL) {
         ssh_remote_error(ssh, "%s", error_msg);
     } else if (ssh->bpp) {
         ssh->bpp->input_eof = true;

ssh/x11fwd.c

@@ -267,12 +267,12 @@ static void x11_log(Plug *p, PlugLogType type, SockAddr *addr, int port,
 static void x11_send_init_error(struct X11Connection *conn,
                                 const char *err_message);
 
-static void x11_closing(Plug *plug, const char *error_msg, int error_code)
+static void x11_closing(Plug *plug, PlugCloseType type, const char *error_msg)
 {
     struct X11Connection *xconn = container_of(
         plug, struct X11Connection, plug);
 
-    if (error_msg) {
+    if (type != PLUGCLOSE_NORMAL) {
         /*
          * Socket error. If we're still at the connection setup stage,
          * construct an X11 error packet passing on the problem.

unix/network.c

@@ -1086,7 +1086,10 @@ void *sk_getxdmdata(Socket *sock, int *lenp)
 
 void plug_closing_errno(Plug *plug, int error)
 {
-    plug_closing(plug, strerror(error), error);
+    PlugCloseType type = PLUGCLOSE_ERROR;
+    if (error == EPIPE)
+        type = PLUGCLOSE_BROKEN_PIPE;
+    plug_closing(plug, type, strerror(error));
 }
 
 /*

unix/pageant.c

@@ -249,7 +249,7 @@ static void x11_log(Plug *p, PlugLogType type, SockAddr *addr, int port,
                     const char *error_msg, int error_code) {}
 static void x11_receive(Plug *plug, int urgent, const char *data, size_t len) {}
 static void x11_sent(Plug *plug, size_t bufsize) {}
-static void x11_closing(Plug *plug, const char *error_msg, int error_code)
+static void x11_closing(Plug *plug, PlugCloseType type, const char *error_msg)
 {
     time_to_die = true;
 }

unix/psusan.c

@@ -273,8 +273,10 @@ static void server_log(Plug *plug, PlugLogType type, SockAddr *addr, int port,
     log_to_stderr(-1, error_msg);
 }
 
-static void server_closing(Plug *plug, const char *error_msg, int error_code)
+static void server_closing(Plug *plug, PlugCloseType type,
+                           const char *error_msg)
 {
+    if (type != PLUGCLOSE_NORMAL)
         log_to_stderr(-1, error_msg);
 }
 

unix/uppity.c

@@ -482,8 +482,10 @@ static void server_log(Plug *plug, PlugLogType type, SockAddr *addr, int port,
     log_to_stderr((unsigned)-1, error_msg);
 }
 
-static void server_closing(Plug *plug, const char *error_msg, int error_code)
+static void server_closing(Plug *plug, PlugCloseType type,
+                           const char *error_msg)
 {
+    if (type != PLUGCLOSE_NORMAL)
         log_to_stderr((unsigned)-1, error_msg);
 }
 

windows/network.c

@@ -1337,12 +1337,15 @@ static void sk_net_close(Socket *sock)
 
 void plug_closing_system_error(Plug *plug, DWORD error)
 {
-    plug_closing(plug, win_strerror(error), error);
+    PlugCloseType type = PLUGCLOSE_ERROR;
+    if (error == ERROR_BROKEN_PIPE)
+        type = PLUGCLOSE_BROKEN_PIPE;
+    plug_closing(plug, type, win_strerror(error));
 }
 
 void plug_closing_winsock_error(Plug *plug, DWORD error)
 {
-    plug_closing(plug, winsock_error_string(error), error);
+    plug_closing(plug, PLUGCLOSE_ERROR, winsock_error_string(error));
 }
 
 /*

windows/platform.h

@@ -121,8 +121,6 @@ static inline uintmax_t strtoumax(const char *nptr, char **endptr, int base)
 #define strnicmp strncasecmp
 #endif
 
-#define BROKEN_PIPE_ERROR_CODE ERROR_BROKEN_PIPE   /* used in ssh/sharing.c */
-
 /*
  * Dynamically linked functions. These come in two flavours:
  *