Escape stuff in the peers accmap as well as our own when async

encoding, and back out the negotiation strategy that didn't
quite have the right effect with some (bad?) ppp implementations.

Drop unexpected PAP/CHAP packets (if not in NETWORK or ESTABLISH
phase).

Make sure that we enter PHASE_ESTABLISH when we need to.

Detect HDLC frames correctly, not just when the whole header
is obtained by a single read().

Don't convert \r\n to \r\r\n in tty raw mode.

Use the correct macros for `', ``'', () and [].

11 files changed, 217 insertions, 129 deletions

diff --git a/usr.sbin/ppp/ppp/async.c b/usr.sbin/ppp/ppp/async.c
index 67447402a79..0c9c8b18976 100644
--- a/usr.sbin/ppp/ppp/async.c
+++ b/usr.sbin/ppp/ppp/async.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: async.c,v 1.2 1999/02/06 03:22:31 brian Exp $
+ * $Id: async.c,v 1.3 1999/04/19 16:59:38 brian Exp $
  *
  */
 #include <sys/types.h>
@@ -57,7 +57,7 @@ void
 async_SetLinkParams(struct async *async, struct lcp *lcp)
 {
   async->my_accmap = lcp->want_accmap;
-  async->his_accmap = lcp->his_accmap;
+  async->his_accmap = lcp->his_accmap | lcp->want_accmap;
 }
 
 /*
diff --git a/usr.sbin/ppp/ppp/chap.c b/usr.sbin/ppp/ppp/chap.c
index 9770a238617..d714281218a 100644
--- a/usr.sbin/ppp/ppp/chap.c
+++ b/usr.sbin/ppp/ppp/chap.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: chap.c,v 1.9 1999/02/20 01:15:12 brian Exp $
+ * $Id: chap.c,v 1.10 1999/04/19 16:59:39 brian Exp $
  *
  *	TODO:
  */
@@ -543,6 +543,13 @@ chap_Input(struct physical *p, struct mbuf *bp)
   int lanman;
 #endif
 
+  if (bundle_Phase(p->dl->bundle) != PHASE_NETWORK &&
+      bundle_Phase(p->dl->bundle) != PHASE_AUTHENTICATE) {
+    log_Printf(LogPHASE, "Unexpected chap input - dropped !\n");
+    mbuf_Free(bp);
+    return;
+  }
+
   if ((bp = auth_ReadHeader(&chap->auth, bp)) == NULL &&
       ntohs(chap->auth.in.hdr.length) == 0)
     log_Printf(LogWARN, "Chap Input: Truncated header !\n");
diff --git a/usr.sbin/ppp/ppp/datalink.c b/usr.sbin/ppp/ppp/datalink.c
index dba15bb8a19..46abc289d6a 100644
--- a/usr.sbin/ppp/ppp/datalink.c
+++ b/usr.sbin/ppp/ppp/datalink.c
@@ -23,7 +23,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- *	$Id: datalink.c,v 1.16 1999/03/04 17:42:25 brian Exp $
+ *	$Id: datalink.c,v 1.17 1999/04/19 16:59:39 brian Exp $
  */
 
 #include <sys/param.h>
@@ -144,6 +144,8 @@ datalink_HangupDone(struct datalink *dl)
     datalink_StartDialTimer(dl, dl->cbcp.fsm.delay);
     cbcp_Down(&dl->cbcp);
     datalink_NewState(dl, DATALINK_OPENING);
+    if (bundle_Phase(dl->bundle) != PHASE_TERMINATE)
+      bundle_NewPhase(dl->bundle, PHASE_ESTABLISH);
   } else if (dl->bundle->CleaningUp ||
       (dl->physical->type == PHYS_DIRECT) ||
       ((!dl->dial.tries || (dl->dial.tries < 0 && !dl->reconnect_tries)) &&
@@ -157,6 +159,8 @@ datalink_HangupDone(struct datalink *dl)
       datalink_StartDialTimer(dl, datalink_GetDialTimeout(dl));
   } else {
     datalink_NewState(dl, DATALINK_OPENING);
+    if (bundle_Phase(dl->bundle) != PHASE_TERMINATE)
+      bundle_NewPhase(dl->bundle, PHASE_ESTABLISH);
     if (dl->dial.tries < 0) {
       datalink_StartDialTimer(dl, dl->cfg.reconnect.timeout);
       dl->dial.tries = dl->cfg.dial.max;
@@ -494,7 +498,7 @@ datalink_LayerUp(void *v, struct fsm *fp)
     lcp->auth_ineed = lcp->want_auth;
     lcp->auth_iwait = lcp->his_auth;
     if (lcp->his_auth || lcp->want_auth) {
-      if (bundle_Phase(dl->bundle) == PHASE_ESTABLISH)
+      if (bundle_Phase(dl->bundle) != PHASE_NETWORK)
         bundle_NewPhase(dl->bundle, PHASE_AUTHENTICATE);
       log_Printf(LogPHASE, "%s: his = %s, mine = %s\n", dl->name,
                 Auth2Nam(lcp->his_auth, lcp->his_authtype),
diff --git a/usr.sbin/ppp/ppp/hdlc.c b/usr.sbin/ppp/ppp/hdlc.c
index 0b3fe0ebbd4..726a89199ee 100644
--- a/usr.sbin/ppp/ppp/hdlc.c
+++ b/usr.sbin/ppp/ppp/hdlc.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: hdlc.c,v 1.4 1999/03/29 08:20:32 brian Exp $
+ * $Id: hdlc.c,v 1.5 1999/04/19 16:59:39 brian Exp $
  *
  *	TODO:
  */
@@ -558,35 +558,41 @@ hdlc_Input(struct bundle *bundle, struct mbuf * bp, struct physical *physical)
 }
 
 /*
- *  Detect a HDLC frame
+ * Detect a HDLC frame
  */
 
-static const char *FrameHeaders[] = {
-  "\176\377\003\300\041",
-  "\176\377\175\043\300\041",
-  "\176\177\175\043\100\041",
-  "\176\175\337\175\043\300\041",
-  "\176\175\137\175\043\100\041",
-  NULL,
+static const struct frameheader {
+  const u_char *data;
+  int len;
+} FrameHeaders[] = {
+  { "\176\377\003\300\041", 5 },
+  { "\176\377\175\043\300\041", 6 },
+  { "\176\177\175\043\100\041", 6 },
+  { "\176\175\337\175\043\300\041", 7 },
+  { "\176\175\137\175\043\100\041", 7 },
+  { NULL, 0 }
 };
 
-u_char *
-hdlc_Detect(struct physical *physical, u_char *cp, int n)
+int
+hdlc_Detect(u_char const **cp, int n, int issync)
 {
-  const char *fp, **hp;
-  char *ptr;
-
-  cp[n] = '\0';				/* be sure to null terminate */
-  ptr = NULL;
-  for (hp = FrameHeaders; *hp; hp++) {
-    fp = *hp;
-    if (physical_IsSync(physical))
-      fp++;
-    ptr = strstr((char *)cp, fp);	/* XXX: cp may have embedded NULs */
-    if (ptr)
-      break;
+  const struct frameheader *fh;
+  const u_char *h;
+  size_t len, cmp;
+
+  while (n) {
+    for (fh = FrameHeaders; fh->len; fh++) {
+      h = issync ? fh->data + 1 : fh->data;
+      len = issync ? fh->len - 1 : fh->len;
+      cmp = n >= len ? len : n;
+      if (memcmp(*cp, h, cmp) == 0)
+        return cmp == len;
+    }
+    n--;
+    (*cp)++;
   }
-  return (u_char *)ptr;
+
+  return 0;
 }
 
 int
diff --git a/usr.sbin/ppp/ppp/hdlc.h b/usr.sbin/ppp/ppp/hdlc.h
index 6e5e412b852..09dc2cf613a 100644
--- a/usr.sbin/ppp/ppp/hdlc.h
+++ b/usr.sbin/ppp/ppp/hdlc.h
@@ -15,7 +15,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: hdlc.h,v 1.2 1999/02/06 03:22:36 brian Exp $
+ * $Id: hdlc.h,v 1.3 1999/04/19 16:59:39 brian Exp $
  *
  *	TODO:
  */
@@ -112,4 +112,4 @@ extern void hdlc_DecodePacket(struct bundle *, u_short, struct mbuf *,
 extern void hdlc_Input(struct bundle *, struct mbuf *, struct physical *);
 extern void hdlc_Output(struct link *, int, u_short, struct mbuf *bp);
 extern u_short hdlc_Fcs(u_short, u_char *, int);
-extern u_char *hdlc_Detect(struct physical *, u_char *, int);
+extern int hdlc_Detect(u_char const **, int, int);
diff --git a/usr.sbin/ppp/ppp/lcp.c b/usr.sbin/ppp/ppp/lcp.c
index da4a0f2cc18..45b091fbfa7 100644
--- a/usr.sbin/ppp/ppp/lcp.c
+++ b/usr.sbin/ppp/ppp/lcp.c
@@ -17,10 +17,8 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: lcp.c,v 1.10 1999/03/29 08:20:44 brian Exp $
+ * $Id: lcp.c,v 1.11 1999/04/19 16:59:39 brian Exp $
  *
- * TODO:
- *	o Limit data field length by MRU
  */
 
 #include <sys/param.h>
@@ -645,42 +643,14 @@ LcpDecodeConfig(struct fsm *fp, u_char *cp, int plen, int mode_type,
       switch (mode_type) {
       case MODE_REQ:
 	lcp->his_accmap = accmap;
-        if ((lcp->want_accmap | accmap) != lcp->want_accmap) {
-          lcp->want_accmap |= accmap;	/* restrict our requested map */
-          lcp->fsm.reqid++;		/* Invalidate the current REQ */
-          /*
-           * If we've already sent a REQ, we want to make sure that
-           * we don't end up sending out a new REQ that doesn't contain
-           * the data that the last one with the same id contained.
-           * This also means that we ignore the peers response to our
-           * last REQ due to an invalid fsm id (even though it's really
-           * correct), probably resulting in a REQ timeout and a resend
-           * with the new accmap and the new id.
-           * If we're already in ST_ACKRCVD at this point, we simply end
-           * up thinking that we negotiated the new accmap - which is ok
-           * as we just end up escaping stuff that the peer probably
-           * can't receive anyway.
-           */
-        }
-        if (lcp->want_accmap == accmap) {
-	  memcpy(dec->ackend, cp, 6);
-	  dec->ackend += 6;
-        } else {
-          /* NAK with what we now want */
-          *dec->nakend++ = *cp;
-          *dec->nakend++ = 6;
-          ua_htonl(&lcp->want_accmap, dec->nakend);
-          dec->nakend += 4;
-        }
+	memcpy(dec->ackend, cp, 6);
+	dec->ackend += 6;
 	break;
       case MODE_NAK:
-	lcp->want_accmap |= accmap;
+	lcp->want_accmap = accmap;
 	break;
       case MODE_REJ:
-        if (lcp->want_accmap)
-          log_Printf(LogWARN, "Peer is rejecting our ACCMAP.... bad news !\n");
-        else
-	  lcp->his_reject |= (1 << type);
+	lcp->his_reject |= (1 << type);
 	break;
       }
       break;
diff --git a/usr.sbin/ppp/ppp/modem.c b/usr.sbin/ppp/ppp/modem.c
index 30077ef4e22..10409be9ed6 100644
--- a/usr.sbin/ppp/ppp/modem.c
+++ b/usr.sbin/ppp/ppp/modem.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: modem.c,v 1.9 1999/03/07 20:57:31 brian Exp $
+ * $Id: modem.c,v 1.10 1999/04/19 16:59:40 brian Exp $
  *
  *  TODO:
  */
@@ -122,7 +122,7 @@ modem_Create(struct datalink *dl, int type)
   p->out = NULL;
   p->connect_count = 0;
   p->dl = dl;
-
+  p->input.sz = 0;
   *p->name.full = '\0';
   p->name.base = p->name.full;
 
@@ -489,6 +489,7 @@ modem_Found(struct physical *modem, struct bundle *bundle)
   throughput_start(&modem->link.throughput, "modem throughput",
                    Enabled(bundle, OPT_THROUGHPUT));
   modem->connect_count++;
+  modem->input.sz = 0;
   log_Printf(LogPHASE, "%s: Connected!\n", modem->link.name);
 }
 
@@ -852,16 +853,15 @@ modem_DescriptorWrite(struct descriptor *d, struct bundle *bundle,
                       const fd_set *fdset)
 {
   struct physical *modem = descriptor2physical(d);
-  int nb, nw, result = 0;
+  int nw, result = 0;
 
   if (modem->out == NULL)
     modem->out = link_Dequeue(&modem->link);
 
   if (modem->out) {
-    nb = modem->out->cnt;
-    nw = physical_Write(modem, MBUF_CTOP(modem->out), nb);
+    nw = physical_Write(modem, MBUF_CTOP(modem->out), modem->out->cnt);
     log_Printf(LogDEBUG, "%s: DescriptorWrite: wrote %d(%d) to %d\n",
-               modem->link.name, nw, nb, modem->fd);
+               modem->link.name, nw, modem->out->cnt, modem->fd);
     if (nw > 0) {
       modem->out->cnt -= nw;
       modem->out->offset += nw;
@@ -963,13 +963,15 @@ modem_DescriptorRead(struct descriptor *d, struct bundle *bundle,
                      const fd_set *fdset)
 {
   struct physical *p = descriptor2physical(d);
-  u_char rbuff[MAX_MRU], *cp;
-  int n;
+  u_char *rbuff;
+  int n, found;
+
+  rbuff = p->input.buf + p->input.sz;
 
   /* something to read from modem */
-  n = physical_Read(p, rbuff, sizeof rbuff);
-  log_Printf(LogDEBUG, "%s: DescriptorRead: read %d from %d\n",
-             p->link.name, n, p->fd);
+  n = physical_Read(p, rbuff, sizeof p->input.buf - p->input.sz);
+  log_Printf(LogDEBUG, "%s: DescriptorRead: read %d/%d from %d\n",
+             p->link.name, n, (int)(sizeof p->input.buf - p->input.sz), p->fd);
   if (n <= 0) {
     if (n < 0)
       log_Printf(LogPHASE, "%s: read (%d): %s\n", p->link.name, p->fd,
@@ -980,23 +982,32 @@ modem_DescriptorRead(struct descriptor *d, struct bundle *bundle,
     datalink_Down(p->dl, CLOSE_NORMAL);
     return;
   }
+
   log_DumpBuff(LogASYNC, "ReadFromModem", rbuff, n);
+  rbuff -= p->input.sz;
+  n += p->input.sz;
 
   if (p->link.lcp.fsm.state <= ST_CLOSED) {
-    /* In -dedicated mode, we just discard input until LCP is started */
     if (p->type != PHYS_DEDICATED) {
-      cp = hdlc_Detect(p, rbuff, n);
-      if (cp) {
+      found = hdlc_Detect((u_char const **)&rbuff, n, physical_IsSync(p));
+      if (rbuff != p->input.buf)
+        log_WritePrompts(p->dl, "%.*s", (int)(rbuff - p->input.buf),
+                         p->input.buf);
+      p->input.sz = n - (rbuff - p->input.buf);
+
+      if (found) {
         /* LCP packet is detected. Turn ourselves into packet mode */
-        if (cp != rbuff)
-          /* Get rid of the bit before the HDLC header */
-          log_WritePrompts(p->dl, "%.*s\r\n", (int)(cp - rbuff), rbuff);
         log_Printf(LogPHASE, "%s: PPP packet detected, coming up\n",
                    p->link.name);
+        log_SetTtyCommandMode(p->dl);
         datalink_Up(p->dl, 0, 1);
+        async_Input(bundle, rbuff, p->input.sz, p);
+        p->input.sz = 0;
       } else
-        log_WritePrompts(p->dl, "%.*s", n, rbuff);
-    }
+        bcopy(rbuff, p->input.buf, p->input.sz);
+    } else
+      /* In -dedicated mode, we just discard input until LCP is started */
+      p->input.sz = 0;
   } else if (n > 0)
     async_Input(bundle, rbuff, n, p);
 }
@@ -1062,6 +1073,7 @@ iov2modem(struct datalink *dl, struct iovec *iov, int *niov, int maxiov, int fd)
 
   throughput_start(&p->link.throughput, "modem throughput",
                    Enabled(dl->bundle, OPT_THROUGHPUT));
+  p->input.sz = 0;
   if (p->Timer.state != TIMER_STOPPED) {
     p->Timer.state = TIMER_STOPPED;	/* Special - see modem2iov() */
     modem_StartTimer(dl->bundle, p);	/* XXX: Should we set cd.required ? */
diff --git a/usr.sbin/ppp/ppp/pap.c b/usr.sbin/ppp/ppp/pap.c
index 0f284c3f892..6d9b8f52b3c 100644
--- a/usr.sbin/ppp/ppp/pap.c
+++ b/usr.sbin/ppp/ppp/pap.c
@@ -18,7 +18,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: pap.c,v 1.6 1999/03/31 14:22:11 brian Exp $
+ * $Id: pap.c,v 1.7 1999/04/19 16:59:40 brian Exp $
  *
  *	TODO:
  */
@@ -156,6 +156,13 @@ pap_Input(struct physical *p, struct mbuf *bp)
   struct authinfo *authp = &p->dl->pap;
   u_char nlen, klen, *key;
 
+  if (bundle_Phase(p->dl->bundle) != PHASE_NETWORK &&
+      bundle_Phase(p->dl->bundle) != PHASE_AUTHENTICATE) {
+    log_Printf(LogPHASE, "Unexpected pap input - dropped !\n");
+    mbuf_Free(bp);
+    return;
+  }
+
   if ((bp = auth_ReadHeader(authp, bp)) == NULL &&
       ntohs(authp->in.hdr.length) == 0) {
     log_Printf(LogWARN, "Pap Input: Truncated header !\n");
diff --git a/usr.sbin/ppp/ppp/physical.h b/usr.sbin/ppp/ppp/physical.h
index 05c753aed6d..fb1ba5e8f97 100644
--- a/usr.sbin/ppp/ppp/physical.h
+++ b/usr.sbin/ppp/ppp/physical.h
@@ -16,7 +16,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- *  $Id: physical.h,v 1.4 1999/02/16 00:18:03 brian Exp $
+ *  $Id: physical.h,v 1.5 1999/04/19 16:59:40 brian Exp $
  *
  */
 
@@ -36,6 +36,11 @@ struct physical {
   struct datalink *dl;         /* my owner */
 
   struct {
+    u_char buf[MAX_MRU];       /* Our input data buffer */
+    size_t sz;
+  } input;
+
+  struct {
     char full[40];
     char *base;
   } name;
diff --git a/usr.sbin/ppp/ppp/ppp.8 b/usr.sbin/ppp/ppp/ppp.8
index 962e789ec08..be6b4916e5e 100644
--- a/usr.sbin/ppp/ppp/ppp.8
+++ b/usr.sbin/ppp/ppp/ppp.8
@@ -1,4 +1,4 @@
-.\" $Id: ppp.8,v 1.35 1999/04/02 15:12:19 aaron Exp $
+.\" $Id: ppp.8,v 1.36 1999/04/19 16:59:40 brian Exp $
 .Dd 20 September 1995
 .nr XX \w'\fC00'
 .Os FreeBSD
@@ -7,7 +7,7 @@
 .Nm ppp
 .Nd Point to Point Protocol (a.k.a. user-ppp) 
 .Sh SYNOPSIS
-.Nm ppp
+.Nm
 .Oo
 .Fl auto |
 .Fl background |
@@ -1604,7 +1604,7 @@ it uses 192.244.177.2.
 The routing table entry will have a netmask of 0xffffffff.
 .El
 .Pp
-This is all fine when each side has a pre-determined IP address.  However,
+This is all fine when each side has a pre-determined IP address, however
 it is often the case that one side is acting as a server which controls
 all IP addresses and the other side should go along with it.  In order
 to allow more flexible behaviour, the
@@ -3248,7 +3248,9 @@ mode with CHAP enabled,
 .Ar id
 is used in the initial authentication challenge and should normally be set to
 the local machine name.
-.It set autoload Ar max-duration max-load [min-duration min-load]
+.It set autoload Xo
+.Ar max-duration max-load Op Ar min-duration min-load
+.Xc
 These settings apply only in multi-link mode and all default to zero.
 When more than one
 .Ar demand-dial
@@ -3278,17 +3280,26 @@ If
 is zero, this timer is disabled.  Because both values default to zero,
 .Ar demand-dial
 links will stay active until the bundle idle timer expires.
-.It set callback [none|auth|cbcp|E.164 *|number[,number]...]...
+.It set callback Ar option Ns No ...
 If no arguments are given, callback is disabled, otherwise,
 .Nm
 will request (or in
 .Fl direct
-mode, will accept) one of the given protocols.  In client mode, if a
-request is NAK'd
-.Nm
-will request another, until no options remain at which point
-.Nm
-will terminate negotiations.  In server mode,
+mode, will accept) one of the given
+.Ar option Ns No s .
+In client mode, if an
+.Ar option
+is NAK'd
+.Nm
+will request a different
+.Ar option ,
+until no options remain at which point
+.Nm
+will terminate negotiations (unless
+.Dq none
+is one of the specified
+.Ar option Ns No ).
+In server mode,
 .Nm
 will accept any of the given protocols - but the client
 .Em must
@@ -3296,7 +3307,9 @@ request one of them.  If you wish callback to be optional, you must include
 .Ar none
 as an option.
 .Pp
-The options are as follows (in this order of preference):
+The
+.Ar option Ns No s
+are as follows (in this order of preference):
 .Pp
 .Bl -tag
 .It auth
@@ -3310,7 +3323,10 @@ the peers entry in
 Microsofts callback control protocol is used.  See
 .Dq set cbcp
 below.
-.It E.164 *|number[,number]...
+.It E.164 *| Ns Xo
+.Ar number Ns Op , Ns Ar number Ns
+.No ...
+.Xc
 The caller specifies the
 .Ar number .
 If
@@ -3336,7 +3352,15 @@ will accept the fact and continue without callback rather than terminating
 the connection.  This is required if you wish callback to be optional.
 .El
 .Pp
-.It set cbcp Op *|number[,number]... Op delay Op retry
+.It set cbcp Oo Xo
+.No *| Ns Ar number Ns No
+.Oo
+.No , Ns Ar number Ns
+.Oc
+.No ...
+.Op Ar delay Op Ar retry
+.Oc
+.Xc
 If no arguments are given, CBCP (Microsofts CallBack Control Protocol)
 is disabled - ie, configuring CBCP in the
 .Dq set callback
@@ -3452,7 +3476,9 @@ command.  Refer to the
 command description above for details.  This command does not affect the
 IP numbers requested using
 .Dq enable dns .
-.It set device|line Ar value[,value...]
+.It set device|line Xo
+.Ar value Ns No ...
+.Xc
 This sets the device(s) to which
 .Nm
 will talk to the given
@@ -3669,7 +3695,17 @@ option above.  It allows the user to specify a set of characters that
 will be
 .Sq escaped
 as they travel across the link.
-.It set filter dial|alive|in|out rule-no permit|deny Ar "[src_addr/width] [dst_addr/width] [proto [src [lt|eq|gt port]] [dst [lt|eq|gt port]] [estab] [syn] [finrst]]"
+.It set filter dial|alive|in|out Ar rule-no Xo
+.No permit|deny
+.Oo Ar src_addr Ns Op / Ns Ar width
+.Op Ar dst_addr Ns Op / Ns Ar width
+.Oc Oo tcp|udp|icmp Op src lt|eq|gt Ar port
+.Op dst lt|eq|gt Ar port
+.Op estab
+.Op syn
+.Op finrst
+.Oc
+.Xc
 .Nm
 supports four filter sets.  The
 .Em alive
@@ -3714,22 +3750,28 @@ be used for devices that fail to reset themselves properly on close.
 This command gives a summary of available set commands, or if
 .Ar command
 is specified, the command usage is shown.
-.It set ifaddr Ar [myaddr [hisaddr [netmask [triggeraddr]]]]
+.It set ifaddr Oo Ar myaddr Ns
+.Op / Ns Ar \&nn
+.Oo Ar hisaddr Ns Op / Ns Ar \&nn
+.Oo Ar netmask
+.Op Ar triggeraddr
+.Oc Oc
+.Oc
 This command specifies the IP addresses that will be used during
 IPCP negotiation.  Addresses are specified using the format
 .Pp
-.Dl a.b.c.d/n
+.Dl a.b.c.d/nn
 .Pp
 Where
-.Ar a.b.c.d
+.Dq a.b.c.d
 is the preferred IP, but
-.Ar n
+.Ar nn
 specifies how many bits of the address we will insist on.  If
-.Ar /n
+.No / Ns Ar nn
 is omitted, it defaults to
-.Ar /32
+.Dq /32
 unless the IP address is 0.0.0.0 in which case it defaults to
-.Ar /0 .
+.Dq /0 .
 .Pp
 .Ar Hisaddr
 may also be specified as a range of IP numbers in the format
@@ -3746,7 +3788,7 @@ for example:
 .Dl set ifaddr 10.0.0.1 10.0.1.2-10.0.1.10,10.0.1.20
 .Pp
 will only negotiate
-.Ar 10.0.0.1
+.Dq 10.0.0.1
 as the local IP number, but may assign any of the given 10 IP
 numbers to the peer.  If the peer requests one of these numbers,
 and that number is not already in use,
@@ -3770,7 +3812,7 @@ range will be accepted.  This is useful when negotiating with some
 .Dv PPP
 implementations that will not assign an IP number unless their peer
 requests
-.Ar 0.0.0.0 .
+.Dq 0.0.0.0 .
 .Pp
 It should be noted that in
 .Fl auto
@@ -3796,11 +3838,21 @@ In all cases, if the interface is already configured,
 .Nm
 will try to maintain the interface IP numbers so that any existing
 bound sockets will remain valid.
-.It "set ccpretry|ccpretries" Ar "[timeout [reqtries [trmtries]]]"
-.It "set chapretry|chapretries" Ar "[timeout [reqtries]]"
-.It "set ipcpretry|ipcpretries" Ar "[timeout [reqtries [trmtries]]]"
-.It "set lcpretry|lcpretries" Ar "[timeout [reqtries [trmtries]]]"
-.It "set papretry|papretries" Ar "[timeout [reqtries]]"
+.It set ccpretry|ccpretries Oo Ar timeout
+.Op Ar reqtries Op Ar trmtries
+.Oc
+.It set chapretry|chapretries Oo Ar timeout
+.Op Ar reqtries
+.Oc
+.It set ipcpretry|ipcpretries Oo Ar timeout
+.Op Ar reqtries Op Ar trmtries
+.Oc
+.It set lcpretry|lcpretries Oo Ar timeout
+.Op Ar reqtries Op Ar trmtries
+.Oc
+.It set papretry|papretries Oo Ar timeout
+.Op Ar reqtries
+.Oc
 These commands set the number of seconds that
 .Nm
 will wait before resending Finite State Machine (FSM) Request packets.
@@ -3831,7 +3883,11 @@ In order to avoid netogiations with the peer that will never converge,
 will only send at most 3 times the configured number of
 .Ar reqtries
 in any given negotiation session before giving up and closing that layer.
-.It set log [local] [+|-] Ns Ar value...
+.It set log Xo
+.Op local
+.Op +|- Ns
+.Ar value Ns No ...
+.Xc
 This command allows the adjustment of the current log level.  Refer
 to the Logging Facility section for further details.
 .It set login Ar chat-script
@@ -3923,13 +3979,24 @@ may be specified here in seconds.
 .It set parity odd|even|none|mark
 This allows the line parity to be set.  The default value is
 .Ar none .
-.It set phone Ar telno[|telno]...[:telno[|telno]...]...
+.It set phone Ar telno Ns Xo
+.Oo \&| Ns Ar backupnumber
+.Oc Ns ... Ns Oo : Ns Ar nextnumber
+.Oc Ns ...
+.Xc
 This allows the specification of the phone number to be used in
 place of the \\\\T string in the dial and login chat scripts.
-Multiple phone numbers may be given separated by a pipe (|) or
-a colon (:).  Numbers after the pipe are only dialed if the dial or login
-script for the previous number failed.  Numbers separated by a colon are
-tried sequentially, irrespective of the reason the line was dropped.
+Multiple phone numbers may be given separated either by a pipe
+.Pq Dq \&|
+or a colon
+.Pq Dq \&: .
+.Pp
+Numbers after the pipe are only dialed if the dial or login
+script for the previous number failed.
+.Pp
+Numbers after the colon are tried sequentially, irrespective of
+the reason the line was dropped.
+.Pp
 If multiple numbers are given,
 .Nm
 will dial them according to these rules until a connection is made, retrying
@@ -3938,7 +4005,9 @@ the maximum number of times specified by
 below.  In
 .Fl background
 mode, each number is attempted at most once.
-.It set [proc]title Op Ar value
+.It set Op proc Ns Xo
+.No title Op Ar value
+.Xc
 The current process title as displayed by
 .Xr ps 1
 is changed according to
@@ -4064,7 +4133,12 @@ This sets the routing table RECVPIPE value.  The optimum value is
 just over twice the MTU value.  If
 .Ar value
 is unspecified or zero, the default kernel controlled value is used.
-.It set redial Ar secs[+inc[-max]][.next] [attempts]
+.It set redial Ar secs Ns Xo
+.Oo + Ns Ar inc Ns
+.Op - Ns Ar max Ns
+.Oc Op . Ns Ar next
+.Op Ar attempts
+.Xc
 .Nm
 can be instructed to attempt to redial
 .Ar attempts
@@ -4167,7 +4241,7 @@ can also be used, but link encryption may be implemented in the future, so
 should not be relied upon.
 .It set speed Ar value
 This sets the speed of the serial device.
-.It set stopped Ar [LCPseconds [CCPseconds]]
+.It set stopped Op Ar LCPseconds Op Ar CCPseconds
 If this option is set,
 .Nm
 will time out after the given FSM (Finite State Machine) has been in
@@ -4260,7 +4334,9 @@ Show the current interface information
 Show the current IPCP statistics.
 .It show lcp
 Show the current LCP statistics.
-.It show [data]link
+.It show Op data Ns Xo
+.No link
+.Xc
 Show high level link information.
 .It show links
 Show a list of available logical links.
diff --git a/usr.sbin/ppp/ppp/prompt.c b/usr.sbin/ppp/ppp/prompt.c
index 3ec1b832f01..e7974d09d32 100644
--- a/usr.sbin/ppp/ppp/prompt.c
+++ b/usr.sbin/ppp/ppp/prompt.c
@@ -23,7 +23,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- *	$Id: prompt.c,v 1.2 1999/02/06 03:22:43 brian Exp $
+ *	$Id: prompt.c,v 1.3 1999/04/19 16:59:42 brian Exp $
  */
 
 #include <sys/param.h>
@@ -380,7 +380,8 @@ prompt_vPrintf(struct prompt *p, const char *fmt, va_list ap)
       /* Stuff '\r' in front of '\n' 'cos we're in raw mode */
       int len = strlen(fmt);
 
-      if (len && len < sizeof nfmt - 1 && fmt[len-1] == '\n') {
+      if (len && len < sizeof nfmt - 1 && fmt[len-1] == '\n' &&
+          (len == 1 || fmt[len-2] != '\r')) {
         strcpy(nfmt, fmt);
         strcpy(nfmt + len - 1, "\r\n");
         pfmt = nfmt;