Have #ip protocol's connect()s throw errors
authorBarret Rhoden <brho@cs.berkeley.edu>
Sat, 13 Feb 2016 17:06:23 +0000 (12:06 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Sun, 14 Feb 2016 14:03:21 +0000 (09:03 -0500)
A couple extra things:

Fsstdbind and Fsstdannounce have temporary waserror shims, until those
get changed.  I needed to change setladdrport to throw, and these needed
to catch it for now.

udpconnect() was the only connect method to call Fsconnected() even if
Fsstdconnect() failed.  All of the others just return immediately.  A
potential effect of Fsconnected() is that it calls rendez_wake on the
"connection rendez" in the conversation (c->cr).  It might be the case
that someone could be sleeping, and we fail to wake them up.  Perhaps a
connect or announce succeeded, but then we failed with this connect, and
now we don't wake anyone.

Given that udpannounce is structured the same as the other connects
(doesn't call Fsconnected() on error), and the qlocks on the CV for
connect and announce, I think this can't happen.  And if it does, then
we just need to fix this connect/announce mess to make it not happen
globally.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/include/ip.h
kern/src/net/devip.c
kern/src/net/icmp.c
kern/src/net/icmp6.c
kern/src/net/ipifc.c
kern/src/net/tcp.c
kern/src/net/udp.c

index 06259f0..f1005ad 100644 (file)
@@ -289,7 +289,7 @@ struct Proto {
        int x;                                          /* protocol index */
        int ipproto;                            /* ip protocol type */
 
-       char *(*connect) (struct conv *, char **unused_char_pp_t, int);
+       void (*connect)(struct conv *, char **, int);
        char *(*announce) (struct conv *, char **unused_char_pp_t, int);
        char *(*bind) (struct conv *, char **unused_char_pp_t, int);
        int (*state) (struct conv *, char *unused_char_p_t, int);
@@ -388,7 +388,7 @@ int Fsbuiltinproto(struct Fs *, uint8_t unused_uint8_t);
 struct conv *Fsprotoclone(struct Proto *, char *unused_char_p_t);
 struct Proto *Fsrcvpcol(struct Fs *, uint8_t unused_uint8_t);
 struct Proto *Fsrcvpcolx(struct Fs *, uint8_t unused_uint8_t);
-char *Fsstdconnect(struct conv *, char **unused_char_pp_t, int);
+void Fsstdconnect(struct conv *, char **, int);
 char *Fsstdannounce(struct conv *, char **unused_char_pp_t, int);
 char *Fsstdbind(struct conv *, char **unused_char_pp_t, int);
 void Fsconvnonblock(struct conv *, bool);
index 88930a6..bc862da 100644 (file)
@@ -848,7 +848,7 @@ static void setladdr(struct conv *c)
 /*
  *  set a local port making sure the quad of raddr,rport,laddr,lport is unique
  */
-static char *setluniqueport(struct conv *c, int lport)
+static void setluniqueport(struct conv *c, int lport)
 {
        struct Proto *p;
        struct conv *xp;
@@ -869,12 +869,11 @@ static char *setluniqueport(struct conv *c, int lport)
                        && ipcmp(xp->raddr, c->raddr) == 0
                        && ipcmp(xp->laddr, c->laddr) == 0) {
                        qunlock(&p->qlock);
-                       return "address in use";
+                       error(EFAIL, "address in use");
                }
        }
        c->lport = lport;
        qunlock(&p->qlock);
-       return NULL;
 }
 
 /*
@@ -928,15 +927,12 @@ static void setlport(struct conv *c)
  *  set a local address and port from a string of the form
  *     [address!]port[!r]
  */
-static char *setladdrport(struct conv *c, char *str, int announcing)
+static void setladdrport(struct conv *c, char *str, int announcing)
 {
        char *p;
-       char *rv;
        uint16_t lport;
        uint8_t addr[IPaddrlen];
 
-       rv = NULL;
-
        /*
         *  ignore restricted part if it exists.  it's
         *  meaningless on local ports.
@@ -963,7 +959,7 @@ static char *setladdrport(struct conv *c, char *str, int announcing)
                        if (ipforme(c->p->f, addr))
                                ipmove(c->laddr, addr);
                        else
-                               return "not a local IP address";
+                               error(EFAIL, "not a local IP address");
                }
        }
 
@@ -971,24 +967,23 @@ static char *setladdrport(struct conv *c, char *str, int announcing)
        if (announcing && strcmp(p, "*") == 0) {
                if (!iseve())
                        error(EPERM, ERROR_FIXME);
-               return setluniqueport(c, 0);
+               setluniqueport(c, 0);
        }
 
        lport = atoi(p);
        if (lport <= 0)
                setlport(c);
        else
-               rv = setluniqueport(c, lport);
-       return rv;
+               setluniqueport(c, lport);
 }
 
-static char *setraddrport(struct conv *c, char *str)
+static void setraddrport(struct conv *c, char *str)
 {
        char *p;
 
        p = strchr(str, '!');
        if (p == NULL)
-               return "malformed address";
+               error(EFAIL, "malformed address");
        *p++ = 0;
        parseip(c->raddr, str);
        c->rport = atoi(p);
@@ -997,33 +992,25 @@ static char *setraddrport(struct conv *c, char *str)
                if (strstr(p, "!r") != NULL)
                        c->restricted = 1;
        }
-       return NULL;
 }
 
 /*
  *  called by protocol connect routine to set addresses
  */
-char *Fsstdconnect(struct conv *c, char *argv[], int argc)
+void Fsstdconnect(struct conv *c, char *argv[], int argc)
 {
-       char *p;
-
        switch (argc) {
                default:
-                       return "bad args to connect";
+                       error(EINVAL, "bad args to %s", __func__);
                case 2:
-                       p = setraddrport(c, argv[1]);
-                       if (p != NULL)
-                               return p;
+                       setraddrport(c, argv[1]);
                        setladdr(c);
                        setlport(c);
                        break;
                case 3:
-                       p = setraddrport(c, argv[1]);
-                       if (p != NULL)
-                               return p;
-                       p = setladdrport(c, argv[2], 0);
-                       if (p != NULL)
-                               return p;
+                       setraddrport(c, argv[1]);
+                       setladdrport(c, argv[2], 0);
+                       break;
        }
 
        if ((memcmp(c->raddr, v4prefix, IPv4off) == 0 &&
@@ -1032,8 +1019,6 @@ char *Fsstdconnect(struct conv *c, char *argv[], int argc)
                c->ipversion = V4;
        else
                c->ipversion = V6;
-
-       return NULL;
 }
 
 /*
@@ -1055,9 +1040,7 @@ static void connectctlmsg(struct Proto *x, struct conv *c, struct cmdbuf *cb)
        c->cerr[0] = '\0';
        if (x->connect == NULL)
                error(EFAIL, "connect not supported");
-       p = x->connect(c, cb->f, cb->nf);
-       if (p != NULL)
-               error(EFAIL, p);
+       x->connect(c, cb->f, cb->nf);
 
        qunlock(&c->qlock);
        if (waserror()) {
@@ -1077,14 +1060,23 @@ static void connectctlmsg(struct Proto *x, struct conv *c, struct cmdbuf *cb)
  */
 char *Fsstdannounce(struct conv *c, char *argv[], int argc)
 {
+       ERRSTACK(1);
+
        memset(c->raddr, 0, sizeof(c->raddr));
        c->rport = 0;
        switch (argc) {
                default:
                        return "bad args to announce";
                case 2:
-                       return setladdrport(c, argv[1], 1);
+                       if (waserror()) {
+                               poperror();
+                               return current_errstr();
+                       }
+                       setladdrport(c, argv[1], 1);
+                       poperror();
+                       break;
        }
+       return 0;
 }
 
 /*
@@ -1128,12 +1120,21 @@ static void announcectlmsg(struct Proto *x, struct conv *c, struct cmdbuf *cb)
  */
 char *Fsstdbind(struct conv *c, char *argv[], int argc)
 {
+       ERRSTACK(1);
+
        switch (argc) {
                default:
                        return "bad args to bind";
                case 2:
-                       return setladdrport(c, argv[1], 0);
+                       if (waserror()) {
+                               poperror();
+                               return current_errstr();
+                       }
+                       setladdrport(c, argv[1], 0);
+                       poperror();
+                       break;
        }
+       return 0;
 }
 
 void Fsconvnonblock(struct conv *cv, bool onoff)
index c125490..b8e3400 100644 (file)
@@ -154,16 +154,10 @@ static void icmpcreate(struct conv *c)
        c->wq = qbypass(icmpkick, c);
 }
 
-extern char *icmpconnect(struct conv *c, char **argv, int argc)
+void icmpconnect(struct conv *c, char **argv, int argc)
 {
-       char *e;
-
-       e = Fsstdconnect(c, argv, argc);
-       if (e != NULL)
-               return e;
-       Fsconnected(c, e);
-
-       return NULL;
+       Fsstdconnect(c, argv, argc);
+       Fsconnected(c, 0);
 }
 
 extern int icmpstate(struct conv *c, char *state, int n)
index b0ec8b9..01948fa 100644 (file)
@@ -942,7 +942,7 @@ int icmpstats6(struct Proto *icmp6, char *buf, int len)
 // need to import from icmp.c
 extern int icmpstate(struct conv *c, char *state, int n);
 extern char *icmpannounce(struct conv *c, char **argv, int argc);
-extern char *icmpconnect(struct conv *c, char **argv, int argc);
+extern void icmpconnect(struct conv *c, char **argv, int argc);
 extern void icmpclose(struct conv *c);
 
 void icmp6newconv(struct Proto *icmp6, struct conv *conv)
index cef9360..df85c0c 100644 (file)
@@ -688,7 +688,7 @@ void ipifcremroute(struct Fs *f, int vers, uint8_t * addr, uint8_t * mask)
  *  addresses.  This is a macro that means, remove all the old interfaces
  *  and add a new one.
  */
-static char *ipifcconnect(struct conv *c, char **argv, int argc)
+static void ipifcconnect(struct conv *c, char **argv, int argc)
 {
        ERRSTACK(1);
        char *err;
@@ -697,7 +697,7 @@ static char *ipifcconnect(struct conv *c, char **argv, int argc)
        ifc = (struct Ipifc *)c->ptcl;
 
        if (ifc->m == NULL)
-               return "ipifc not yet bound to device";
+               error(EFAIL, "ipifc not yet bound to device");
 
        wlock(&ifc->rwlock);
        if (waserror()) {
@@ -709,16 +709,9 @@ static char *ipifcconnect(struct conv *c, char **argv, int argc)
        wunlock(&ifc->rwlock);
        poperror();
 
-       if (waserror()) {
-               poperror();
-               return current_errstr();
-       }
        ipifcadd(ifc, argv, argc, 0, NULL);
-       poperror();
 
        Fsconnected(c, NULL);
-
-       return NULL;
 }
 
 static void ipifcsetpar6(struct Ipifc *ifc, char **argv, int argc)
index 2ae15b7..8ae2be2 100644 (file)
@@ -463,16 +463,10 @@ void tcpsetstate(struct conv *s, uint8_t newstate)
                Fsconnected(s, NULL);
 }
 
-static char *tcpconnect(struct conv *c, char **argv, int argc)
+static void tcpconnect(struct conv *c, char **argv, int argc)
 {
-       char *e;
-
-       e = Fsstdconnect(c, argv, argc);
-       if (e != NULL)
-               return e;
+       Fsstdconnect(c, argv, argc);
        tcpstart(c, TCP_CONNECT);
-
-       return NULL;
 }
 
 static int tcpstate(struct conv *c, char *state, int n)
index 8b280de..001515f 100644 (file)
@@ -146,19 +146,15 @@ struct Udpcb {
        uint8_t headers;
 };
 
-static char *udpconnect(struct conv *c, char **argv, int argc)
+static void udpconnect(struct conv *c, char **argv, int argc)
 {
-       char *e;
        Udppriv *upriv;
 
        upriv = c->p->priv;
-       e = Fsstdconnect(c, argv, argc);
-       Fsconnected(c, e);
-       if (e != NULL)
-               return e;
+       Fsstdconnect(c, argv, argc);
+       Fsconnected(c, 0);
 
        iphtadd(&upriv->ht, c);
-       return NULL;
 }
 
 static int udpstate(struct conv *c, char *state, int n)