Option for alternative escape character with cu(1)

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

Option for alternative escape character with cu(1)

Artturi Alm
Hi,

i don't have issues with tilde when using locally, but i mostly ssh to
reach cu, and too many times i've forgotten to configure ssh/use -e,
with this cu(1) becomes safer/easier to use for us with non-english
keyboard.
~tilde is certainly annoying when it's three key presses alone,
and then you mostly get only one shot at trying..

is this bloat?

-Artturi


diff --git a/usr.bin/cu/command.c b/usr.bin/cu/command.c
index c07fe73aeca..d97db3b56de 100644
--- a/usr.bin/cu/command.c
+++ b/usr.bin/cu/command.c
@@ -223,6 +223,8 @@ start_record(void)
 void
 do_command(char c)
 {
+ char esc = alt_esc ? '%' : '~';
+
  if (restricted && strchr("CRX$>", c) != NULL) {
  cu_warnx("~%c command is not allowed in restricted mode", c);
  return;
@@ -271,15 +273,16 @@ do_command(char c)
  break;
  case '?':
  printf("\r\n"
-    "~#      send break\r\n"
-    "~$      pipe local command to remote host\r\n"
-    "~>      send file to remote host\r\n"
-    "~C      connect program to remote host\r\n"
-    "~D      de-assert DTR line briefly\r\n"
-    "~R      start recording to file\r\n"
-    "~S      set speed\r\n"
-    "~X      send file with XMODEM\r\n"
-    "~?      get this summary\r\n"
+    "%c#      send break\r\n"
+    "%c$      pipe local command to remote host\r\n"
+    "%c>      send file to remote host\r\n"
+    "%cC      connect program to remote host\r\n"
+    "%cD      de-assert DTR line briefly\r\n"
+    "%cR      start recording to file\r\n"
+    "%cS      set speed\r\n"
+    "%cX      send file with XMODEM\r\n"
+    "%c?      get this summary\r\n",
+    esc, esc, esc, esc, esc, esc, esc, esc, esc
  );
  break;
  }
diff --git a/usr.bin/cu/cu.1 b/usr.bin/cu/cu.1
index 104a6ea7893..1d609e14947 100644
--- a/usr.bin/cu/cu.1
+++ b/usr.bin/cu/cu.1
@@ -35,7 +35,7 @@
 .Nd serial terminal emulator
 .Sh SYNOPSIS
 .Nm
-.Op Fl dr
+.Op Fl der
 .Op Fl l Ar line
 .Op Fl s Ar speed | Fl Ar speed
 .Nm
@@ -55,6 +55,10 @@ The options are as follows:
 Specify that the line is directly connected and
 .Nm
 should not allow the driver to block waiting for a carrier to be detected.
+.It Fl e
+Use a percent sign
+.Pq Ql %
+as the escape character instead of tilde.
 .It Fl l Ar line
 Specify the line to use.
 Either of the forms like
diff --git a/usr.bin/cu/cu.c b/usr.bin/cu/cu.c
index 03a2df4181f..b66f4698605 100644
--- a/usr.bin/cu/cu.c
+++ b/usr.bin/cu/cu.c
@@ -41,6 +41,7 @@ FILE *record_file;
 struct termios saved_tio;
 struct bufferevent *input_ev;
 struct bufferevent *output_ev;
+int alt_esc = 0;
 int is_direct = -1;
 int restricted = 0;
 const char *line_path = NULL;
@@ -53,7 +54,7 @@ struct event sighup_ev;
 enum {
  STATE_NONE,
  STATE_NEWLINE,
- STATE_TILDE
+ STATE_ESCAPE
 } last_state = STATE_NEWLINE;
 
 __dead void usage(void);
@@ -67,7 +68,7 @@ void try_remote(const char *, const char *, const char *);
 __dead void
 usage(void)
 {
- fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]\n",
+ fprintf(stderr, "usage: %s [-der] [-l line] [-s speed | -speed]\n",
     __progname);
  fprintf(stderr, "       %s [host]\n", __progname);
  exit(1);
@@ -101,11 +102,14 @@ main(int argc, char **argv)
  errx(1, "speed asprintf");
  }
 
- while ((opt = getopt(argc, argv, "drl:s:")) != -1) {
+ while ((opt = getopt(argc, argv, "derl:s:")) != -1) {
  switch (opt) {
  case 'd':
  is_direct = 1;
  break;
+ case 'e':
+ alt_esc = 1;
+ break;
  case 'r':
  if (pledge("stdio rpath wpath tty", NULL) == -1)
  err(1, "pledge");
@@ -308,14 +312,14 @@ stream_read(struct bufferevent *bufev, void *data)
  last_state = STATE_NEWLINE;
  break;
  case STATE_NEWLINE:
- if (state_change && *ptr == '~') {
- last_state = STATE_TILDE;
+ if (state_change && *ptr == "~%"[alt_esc]) {
+ last_state = STATE_ESCAPE;
  continue;
  }
  if (*ptr != '\r')
  last_state = STATE_NONE;
  break;
- case STATE_TILDE:
+ case STATE_ESCAPE:
  do_command(*ptr);
  last_state = STATE_NEWLINE;
  continue;
diff --git a/usr.bin/cu/cu.h b/usr.bin/cu/cu.h
index 2a7ca45d414..9d8ea3fc86a 100644
--- a/usr.bin/cu/cu.h
+++ b/usr.bin/cu/cu.h
@@ -23,6 +23,7 @@
 void do_command(char);
 
 /* cu.c */
+extern int alt_esc;
 extern int restricted;
 extern FILE *record_file;
 extern struct termios saved_tio;

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Todd C. Miller-3
On Wed, 13 Mar 2019 14:35:06 +0200, Artturi Alm wrote:

> i don't have issues with tilde when using locally, but i mostly ssh to
> reach cu, and too many times i've forgotten to configure ssh/use -e,
> with this cu(1) becomes safer/easier to use for us with non-english
> keyboard.
> ~tilde is certainly annoying when it's three key presses alone,
> and then you mostly get only one shot at trying..

The cu from Taylor UUCP uses -E for this (it uses -e to specify
even parity).  If we are going to support this, I think it makes
sense to be as compatible as possible with other systems.

 - todd

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Nicholas Marriott-2
In reply to this post by Artturi Alm
Why only % rather than have -e take an argument like ssh?


On Wed, Mar 13, 2019 at 02:35:06PM +0200, Artturi Alm wrote:

> Hi,
>
> i don't have issues with tilde when using locally, but i mostly ssh to
> reach cu, and too many times i've forgotten to configure ssh/use -e,
> with this cu(1) becomes safer/easier to use for us with non-english
> keyboard.
> ~tilde is certainly annoying when it's three key presses alone,
> and then you mostly get only one shot at trying..
>
> is this bloat?
>
> -Artturi
>
>
> diff --git a/usr.bin/cu/command.c b/usr.bin/cu/command.c
> index c07fe73aeca..d97db3b56de 100644
> --- a/usr.bin/cu/command.c
> +++ b/usr.bin/cu/command.c
> @@ -223,6 +223,8 @@ start_record(void)
>  void
>  do_command(char c)
>  {
> + char esc = alt_esc ? '%' : '~';
> +
>   if (restricted && strchr("CRX$>", c) != NULL) {
>   cu_warnx("~%c command is not allowed in restricted mode", c);
>   return;
> @@ -271,15 +273,16 @@ do_command(char c)
>   break;
>   case '?':
>   printf("\r\n"
> -    "~#      send break\r\n"
> -    "~$      pipe local command to remote host\r\n"
> -    "~>      send file to remote host\r\n"
> -    "~C      connect program to remote host\r\n"
> -    "~D      de-assert DTR line briefly\r\n"
> -    "~R      start recording to file\r\n"
> -    "~S      set speed\r\n"
> -    "~X      send file with XMODEM\r\n"
> -    "~?      get this summary\r\n"
> +    "%c#      send break\r\n"
> +    "%c$      pipe local command to remote host\r\n"
> +    "%c>      send file to remote host\r\n"
> +    "%cC      connect program to remote host\r\n"
> +    "%cD      de-assert DTR line briefly\r\n"
> +    "%cR      start recording to file\r\n"
> +    "%cS      set speed\r\n"
> +    "%cX      send file with XMODEM\r\n"
> +    "%c?      get this summary\r\n",
> +    esc, esc, esc, esc, esc, esc, esc, esc, esc
>   );
>   break;
>   }
> diff --git a/usr.bin/cu/cu.1 b/usr.bin/cu/cu.1
> index 104a6ea7893..1d609e14947 100644
> --- a/usr.bin/cu/cu.1
> +++ b/usr.bin/cu/cu.1
> @@ -35,7 +35,7 @@
>  .Nd serial terminal emulator
>  .Sh SYNOPSIS
>  .Nm
> -.Op Fl dr
> +.Op Fl der
>  .Op Fl l Ar line
>  .Op Fl s Ar speed | Fl Ar speed
>  .Nm
> @@ -55,6 +55,10 @@ The options are as follows:
>  Specify that the line is directly connected and
>  .Nm
>  should not allow the driver to block waiting for a carrier to be detected.
> +.It Fl e
> +Use a percent sign
> +.Pq Ql %
> +as the escape character instead of tilde.
>  .It Fl l Ar line
>  Specify the line to use.
>  Either of the forms like
> diff --git a/usr.bin/cu/cu.c b/usr.bin/cu/cu.c
> index 03a2df4181f..b66f4698605 100644
> --- a/usr.bin/cu/cu.c
> +++ b/usr.bin/cu/cu.c
> @@ -41,6 +41,7 @@ FILE *record_file;
>  struct termios saved_tio;
>  struct bufferevent *input_ev;
>  struct bufferevent *output_ev;
> +int alt_esc = 0;
>  int is_direct = -1;
>  int restricted = 0;
>  const char *line_path = NULL;
> @@ -53,7 +54,7 @@ struct event sighup_ev;
>  enum {
>   STATE_NONE,
>   STATE_NEWLINE,
> - STATE_TILDE
> + STATE_ESCAPE
>  } last_state = STATE_NEWLINE;
>  
>  __dead void usage(void);
> @@ -67,7 +68,7 @@ void try_remote(const char *, const char *, const char *);
>  __dead void
>  usage(void)
>  {
> - fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]\n",
> + fprintf(stderr, "usage: %s [-der] [-l line] [-s speed | -speed]\n",
>      __progname);
>   fprintf(stderr, "       %s [host]\n", __progname);
>   exit(1);
> @@ -101,11 +102,14 @@ main(int argc, char **argv)
>   errx(1, "speed asprintf");
>   }
>  
> - while ((opt = getopt(argc, argv, "drl:s:")) != -1) {
> + while ((opt = getopt(argc, argv, "derl:s:")) != -1) {
>   switch (opt) {
>   case 'd':
>   is_direct = 1;
>   break;
> + case 'e':
> + alt_esc = 1;
> + break;
>   case 'r':
>   if (pledge("stdio rpath wpath tty", NULL) == -1)
>   err(1, "pledge");
> @@ -308,14 +312,14 @@ stream_read(struct bufferevent *bufev, void *data)
>   last_state = STATE_NEWLINE;
>   break;
>   case STATE_NEWLINE:
> - if (state_change && *ptr == '~') {
> - last_state = STATE_TILDE;
> + if (state_change && *ptr == "~%"[alt_esc]) {
> + last_state = STATE_ESCAPE;
>   continue;
>   }
>   if (*ptr != '\r')
>   last_state = STATE_NONE;
>   break;
> - case STATE_TILDE:
> + case STATE_ESCAPE:
>   do_command(*ptr);
>   last_state = STATE_NEWLINE;
>   continue;
> diff --git a/usr.bin/cu/cu.h b/usr.bin/cu/cu.h
> index 2a7ca45d414..9d8ea3fc86a 100644
> --- a/usr.bin/cu/cu.h
> +++ b/usr.bin/cu/cu.h
> @@ -23,6 +23,7 @@
>  void do_command(char);
>  
>  /* cu.c */
> +extern int alt_esc;
>  extern int restricted;
>  extern FILE *record_file;
>  extern struct termios saved_tio;

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Artturi Alm
In reply to this post by Todd C. Miller-3
On Wed, Mar 13, 2019 at 07:47:08AM -0600, Todd C. Miller wrote:

> On Wed, 13 Mar 2019 14:35:06 +0200, Artturi Alm wrote:
>
> > i don't have issues with tilde when using locally, but i mostly ssh to
> > reach cu, and too many times i've forgotten to configure ssh/use -e,
> > with this cu(1) becomes safer/easier to use for us with non-english
> > keyboard.
> > ~tilde is certainly annoying when it's three key presses alone,
> > and then you mostly get only one shot at trying..
>
> The cu from Taylor UUCP uses -E for this (it uses -e to specify
> even parity).  If we are going to support this, I think it makes
> sense to be as compatible as possible with other systems.
>
>  - todd

Sure, makes sense to me, thanks.


On Wed, Mar 13, 2019 at 02:36:03PM +0000, Nicholas Marriott wrote:
> Why only % rather than have -e take an argument like ssh?
>

I'm lazy, and wanted to avoid breaking the "line" in usage().. :]

i had also missed escaping the alternative escape character in
do_command(), diff with these things fixed below.

-Artturi


diff --git a/usr.bin/cu/command.c b/usr.bin/cu/command.c
index c07fe73aeca..e225fb544be 100644
--- a/usr.bin/cu/command.c
+++ b/usr.bin/cu/command.c
@@ -30,6 +30,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <vis.h>
 
 #include "cu.h"
 
@@ -223,6 +224,8 @@ start_record(void)
 void
 do_command(char c)
 {
+ char esc[4 + 1];
+
  if (restricted && strchr("CRX$>", c) != NULL) {
  cu_warnx("~%c command is not allowed in restricted mode", c);
  return;
@@ -266,20 +269,26 @@ do_command(char c)
  sleep(1);
  ioctl(line_fd, TIOCCBRK, NULL);
  break;
- case '~':
- bufferevent_write(line_ev, "~", 1);
+ default:
+ if (c != escape_char)
+ break;
+ esc[0] = escape_char;
+ esc[1] = '\0';
+ bufferevent_write(line_ev, esc, 1);
  break;
  case '?':
+ vis(esc, escape_char, VIS_WHITE | VIS_NOSLASH, 0);
  printf("\r\n"
-    "~#      send break\r\n"
-    "~$      pipe local command to remote host\r\n"
-    "~>      send file to remote host\r\n"
-    "~C      connect program to remote host\r\n"
-    "~D      de-assert DTR line briefly\r\n"
-    "~R      start recording to file\r\n"
-    "~S      set speed\r\n"
-    "~X      send file with XMODEM\r\n"
-    "~?      get this summary\r\n"
+    "%s#      send break\r\n"
+    "%s$      pipe local command to remote host\r\n"
+    "%s>      send file to remote host\r\n"
+    "%sC      connect program to remote host\r\n"
+    "%sD      de-assert DTR line briefly\r\n"
+    "%sR      start recording to file\r\n"
+    "%sS      set speed\r\n"
+    "%sX      send file with XMODEM\r\n"
+    "%s?      get this summary\r\n",
+    esc, esc, esc, esc, esc, esc, esc, esc, esc
  );
  break;
  }
diff --git a/usr.bin/cu/cu.1 b/usr.bin/cu/cu.1
index 104a6ea7893..d52e0b94e5f 100644
--- a/usr.bin/cu/cu.1
+++ b/usr.bin/cu/cu.1
@@ -38,6 +38,7 @@
 .Op Fl dr
 .Op Fl l Ar line
 .Op Fl s Ar speed | Fl Ar speed
+.Op Fl E Ar escape_char
 .Nm
 .Op Ar host
 .Sh DESCRIPTION
@@ -92,6 +93,8 @@ and
 .It Fl s Ar speed | Fl Ar speed
 Set the speed of the connection.
 The default is 9600.
+.It Fl E Ar escape_char
+Specify a escape character to use instead of the default tilde.
 .El
 .Pp
 If
diff --git a/usr.bin/cu/cu.c b/usr.bin/cu/cu.c
index 03a2df4181f..f269f4a74f3 100644
--- a/usr.bin/cu/cu.c
+++ b/usr.bin/cu/cu.c
@@ -41,6 +41,7 @@ FILE *record_file;
 struct termios saved_tio;
 struct bufferevent *input_ev;
 struct bufferevent *output_ev;
+int escape_char = -1;
 int is_direct = -1;
 int restricted = 0;
 const char *line_path = NULL;
@@ -53,7 +54,7 @@ struct event sighup_ev;
 enum {
  STATE_NONE,
  STATE_NEWLINE,
- STATE_TILDE
+ STATE_ESCAPE
 } last_state = STATE_NEWLINE;
 
 __dead void usage(void);
@@ -67,8 +68,8 @@ void try_remote(const char *, const char *, const char *);
 __dead void
 usage(void)
 {
- fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]\n",
-    __progname);
+ fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]"
+    " [-E escape_char]\n", __progname);
  fprintf(stderr, "       %s [host]\n", __progname);
  exit(1);
 }
@@ -101,7 +102,7 @@ main(int argc, char **argv)
  errx(1, "speed asprintf");
  }
 
- while ((opt = getopt(argc, argv, "drl:s:")) != -1) {
+ while ((opt = getopt(argc, argv, "drl:s:E:")) != -1) {
  switch (opt) {
  case 'd':
  is_direct = 1;
@@ -119,6 +120,17 @@ main(int argc, char **argv)
  if (errstr != NULL)
  errx(1, "speed is %s: %s", errstr, optarg);
  break;
+ case 'E':
+ if (optarg[0] == '^' && optarg[2] == 0 &&
+    (unsigned char)optarg[1] >= 64 &&
+    (unsigned char)optarg[1] < 128)
+ escape_char = (unsigned char)optarg[1] & 31;
+ else if (strlen(optarg) == 1)
+ escape_char = (unsigned char)optarg[0];
+ else
+ errx(1, "invalid escape character: %s",
+    optarg);
+ break;
  default:
  usage();
  }
@@ -155,6 +167,8 @@ main(int argc, char **argv)
  line_speed = 9600;
  if (is_direct == -1)
  is_direct = 0;
+ if (escape_char == -1)
+ escape_char = '~';
 
  if (strchr(line_path, '/') == NULL) {
  if (asprintf(&tmp, "%s%s", _PATH_DEV, line_path) == -1)
@@ -308,14 +322,14 @@ stream_read(struct bufferevent *bufev, void *data)
  last_state = STATE_NEWLINE;
  break;
  case STATE_NEWLINE:
- if (state_change && *ptr == '~') {
- last_state = STATE_TILDE;
+ if (state_change && *ptr == escape_char) {
+ last_state = STATE_ESCAPE;
  continue;
  }
  if (*ptr != '\r')
  last_state = STATE_NONE;
  break;
- case STATE_TILDE:
+ case STATE_ESCAPE:
  do_command(*ptr);
  last_state = STATE_NEWLINE;
  continue;
diff --git a/usr.bin/cu/cu.h b/usr.bin/cu/cu.h
index 2a7ca45d414..abcedc6d77a 100644
--- a/usr.bin/cu/cu.h
+++ b/usr.bin/cu/cu.h
@@ -23,6 +23,7 @@
 void do_command(char);
 
 /* cu.c */
+extern int escape_char;
 extern int restricted;
 extern FILE *record_file;
 extern struct termios saved_tio;

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Nicholas Marriott-2
Thanks, comments inline.

> diff --git a/usr.bin/cu/command.c b/usr.bin/cu/command.c
> index c07fe73aeca..e225fb544be 100644
> --- a/usr.bin/cu/command.c
> +++ b/usr.bin/cu/command.c
> @@ -30,6 +30,7 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include <vis.h>
>  
>  #include "cu.h"
>  
> @@ -223,6 +224,8 @@ start_record(void)
>  void
>  do_command(char c)
>  {
> + char esc[4 + 1];
> +
>   if (restricted && strchr("CRX$>", c) != NULL) {
>   cu_warnx("~%c command is not allowed in restricted mode", c);
>   return;
> @@ -266,20 +269,26 @@ do_command(char c)
>   sleep(1);
>   ioctl(line_fd, TIOCCBRK, NULL);
>   break;
> - case '~':
> - bufferevent_write(line_ev, "~", 1);
> + default:
> + if (c != escape_char)
> + break;
> + esc[0] = escape_char;
> + esc[1] = '\0';

No need for \0 since you are only writing one byte, but...

> + bufferevent_write(line_ev, esc, 1);

...why not do this instead:

    bufferevent_write(line_ev, &c, 1);

>   break;
>   case '?':
> + vis(esc, escape_char, VIS_WHITE | VIS_NOSLASH, 0);
>   printf("\r\n"
> -    "~#      send break\r\n"
> -    "~$      pipe local command to remote host\r\n"
> -    "~>      send file to remote host\r\n"
> -    "~C      connect program to remote host\r\n"
> -    "~D      de-assert DTR line briefly\r\n"
> -    "~R      start recording to file\r\n"
> -    "~S      set speed\r\n"
> -    "~X      send file with XMODEM\r\n"
> -    "~?      get this summary\r\n"
> +    "%s#      send break\r\n"
> +    "%s$      pipe local command to remote host\r\n"
> +    "%s>      send file to remote host\r\n"
> +    "%sC      connect program to remote host\r\n"
> +    "%sD      de-assert DTR line briefly\r\n"
> +    "%sR      start recording to file\r\n"
> +    "%sS      set speed\r\n"
> +    "%sX      send file with XMODEM\r\n"
> +    "%s?      get this summary\r\n",
> +    esc, esc, esc, esc, esc, esc, esc, esc, esc
>   );
>   break;
>   }
> diff --git a/usr.bin/cu/cu.1 b/usr.bin/cu/cu.1
> index 104a6ea7893..d52e0b94e5f 100644
> --- a/usr.bin/cu/cu.1
> +++ b/usr.bin/cu/cu.1
> @@ -38,6 +38,7 @@
>  .Op Fl dr
>  .Op Fl l Ar line
>  .Op Fl s Ar speed | Fl Ar speed
> +.Op Fl E Ar escape_char

E comes before l and s.

>  .Nm
>  .Op Ar host
>  .Sh DESCRIPTION
> @@ -92,6 +93,8 @@ and
>  .It Fl s Ar speed | Fl Ar speed
>  Set the speed of the connection.
>  The default is 9600.
> +.It Fl E Ar escape_char
> +Specify a escape character to use instead of the default tilde.

"an escape character".

>  .El
>  .Pp
>  If
> diff --git a/usr.bin/cu/cu.c b/usr.bin/cu/cu.c
> index 03a2df4181f..f269f4a74f3 100644
> --- a/usr.bin/cu/cu.c
> +++ b/usr.bin/cu/cu.c
> @@ -41,6 +41,7 @@ FILE *record_file;
>  struct termios saved_tio;
>  struct bufferevent *input_ev;
>  struct bufferevent *output_ev;
> +int escape_char = -1;

Why not just initialize it to '~'?

>  int is_direct = -1;
>  int restricted = 0;
>  const char *line_path = NULL;
> @@ -53,7 +54,7 @@ struct event sighup_ev;
>  enum {
>   STATE_NONE,
>   STATE_NEWLINE,
> - STATE_TILDE
> + STATE_ESCAPE
>  } last_state = STATE_NEWLINE;
>  
>  __dead void usage(void);
> @@ -67,8 +68,8 @@ void try_remote(const char *, const char *, const char *);
>  __dead void
>  usage(void)
>  {
> - fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]\n",
> -    __progname);
> + fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]"
> +    " [-E escape_char]\n", __progname);

-E comes before -l and -s.

>   fprintf(stderr, "       %s [host]\n", __progname);
>   exit(1);
>  }
> @@ -101,7 +102,7 @@ main(int argc, char **argv)
>   errx(1, "speed asprintf");
>   }
>  
> - while ((opt = getopt(argc, argv, "drl:s:")) != -1) {
> + while ((opt = getopt(argc, argv, "drl:s:E:")) != -1) {
>   switch (opt) {
>   case 'd':
>   is_direct = 1;
> @@ -119,6 +120,17 @@ main(int argc, char **argv)
>   if (errstr != NULL)
>   errx(1, "speed is %s: %s", errstr, optarg);
>   break;
> + case 'E':
> + if (optarg[0] == '^' && optarg[2] == 0 &&
> +    (unsigned char)optarg[1] >= 64 &&
> +    (unsigned char)optarg[1] < 128)
> + escape_char = (unsigned char)optarg[1] & 31;
> + else if (strlen(optarg) == 1)
> + escape_char = (unsigned char)optarg[0];
> + else

I know you copied this from ssh, but please use u_char rather than
unsigned char like cu does elsewhere.

> + errx(1, "invalid escape character: %s",
> +    optarg);
> + break;
>   default:
>   usage();
>   }
> @@ -155,6 +167,8 @@ main(int argc, char **argv)
>   line_speed = 9600;
>   if (is_direct == -1)
>   is_direct = 0;
> + if (escape_char == -1)
> + escape_char = '~';
>  
>   if (strchr(line_path, '/') == NULL) {
>   if (asprintf(&tmp, "%s%s", _PATH_DEV, line_path) == -1)
> @@ -308,14 +322,14 @@ stream_read(struct bufferevent *bufev, void *data)
>   last_state = STATE_NEWLINE;
>   break;
>   case STATE_NEWLINE:
> - if (state_change && *ptr == '~') {
> - last_state = STATE_TILDE;
> + if (state_change && *ptr == escape_char) {
> + last_state = STATE_ESCAPE;
>   continue;
>   }
>   if (*ptr != '\r')
>   last_state = STATE_NONE;
>   break;
> - case STATE_TILDE:
> + case STATE_ESCAPE:
>   do_command(*ptr);
>   last_state = STATE_NEWLINE;
>   continue;
> diff --git a/usr.bin/cu/cu.h b/usr.bin/cu/cu.h
> index 2a7ca45d414..abcedc6d77a 100644
> --- a/usr.bin/cu/cu.h
> +++ b/usr.bin/cu/cu.h
> @@ -23,6 +23,7 @@
>  void do_command(char);
>  
>  /* cu.c */
> +extern int escape_char;
>  extern int restricted;
>  extern FILE *record_file;
>  extern struct termios saved_tio;
>

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Artturi Alm
On Thu, Mar 14, 2019 at 10:18:57AM +0000, Nicholas Marriott wrote:
> Thanks, comments inline.
>

The diff looks much better to me, now with those things fixed based
on your feedback, thanks :]

-Artturi


diff --git a/usr.bin/cu/command.c b/usr.bin/cu/command.c
index c07fe73aeca..27d80f16dd7 100644
--- a/usr.bin/cu/command.c
+++ b/usr.bin/cu/command.c
@@ -30,6 +30,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <vis.h>
 
 #include "cu.h"
 
@@ -223,6 +224,8 @@ start_record(void)
 void
 do_command(char c)
 {
+ char esc[4 + 1];
+
  if (restricted && strchr("CRX$>", c) != NULL) {
  cu_warnx("~%c command is not allowed in restricted mode", c);
  return;
@@ -266,20 +269,23 @@ do_command(char c)
  sleep(1);
  ioctl(line_fd, TIOCCBRK, NULL);
  break;
- case '~':
- bufferevent_write(line_ev, "~", 1);
+ default:
+ if (c == escape_char)
+ bufferevent_write(line_ev, &c, 1);
  break;
  case '?':
+ vis(esc, escape_char, VIS_WHITE | VIS_NOSLASH, 0);
  printf("\r\n"
-    "~#      send break\r\n"
-    "~$      pipe local command to remote host\r\n"
-    "~>      send file to remote host\r\n"
-    "~C      connect program to remote host\r\n"
-    "~D      de-assert DTR line briefly\r\n"
-    "~R      start recording to file\r\n"
-    "~S      set speed\r\n"
-    "~X      send file with XMODEM\r\n"
-    "~?      get this summary\r\n"
+    "%s#      send break\r\n"
+    "%s$      pipe local command to remote host\r\n"
+    "%s>      send file to remote host\r\n"
+    "%sC      connect program to remote host\r\n"
+    "%sD      de-assert DTR line briefly\r\n"
+    "%sR      start recording to file\r\n"
+    "%sS      set speed\r\n"
+    "%sX      send file with XMODEM\r\n"
+    "%s?      get this summary\r\n",
+    esc, esc, esc, esc, esc, esc, esc, esc, esc
  );
  break;
  }
diff --git a/usr.bin/cu/cu.1 b/usr.bin/cu/cu.1
index 104a6ea7893..3e85488e87c 100644
--- a/usr.bin/cu/cu.1
+++ b/usr.bin/cu/cu.1
@@ -36,6 +36,7 @@
 .Sh SYNOPSIS
 .Nm
 .Op Fl dr
+.Op Fl E Ar escape_char
 .Op Fl l Ar line
 .Op Fl s Ar speed | Fl Ar speed
 .Nm
@@ -55,6 +56,8 @@ The options are as follows:
 Specify that the line is directly connected and
 .Nm
 should not allow the driver to block waiting for a carrier to be detected.
+.It Fl E Ar escape_char
+Specify an escape character to use instead of the default tilde.
 .It Fl l Ar line
 Specify the line to use.
 Either of the forms like
diff --git a/usr.bin/cu/cu.c b/usr.bin/cu/cu.c
index 03a2df4181f..8e3ce0b0c0e 100644
--- a/usr.bin/cu/cu.c
+++ b/usr.bin/cu/cu.c
@@ -41,6 +41,7 @@ FILE *record_file;
 struct termios saved_tio;
 struct bufferevent *input_ev;
 struct bufferevent *output_ev;
+int escape_char = '~';
 int is_direct = -1;
 int restricted = 0;
 const char *line_path = NULL;
@@ -53,7 +54,7 @@ struct event sighup_ev;
 enum {
  STATE_NONE,
  STATE_NEWLINE,
- STATE_TILDE
+ STATE_ESCAPE
 } last_state = STATE_NEWLINE;
 
 __dead void usage(void);
@@ -67,8 +68,8 @@ void try_remote(const char *, const char *, const char *);
 __dead void
 usage(void)
 {
- fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]\n",
-    __progname);
+ fprintf(stderr, "usage: %s [-dr] [-E escape_char] [-l line] "
+    "[-s speed | -speed]\n", __progname);
  fprintf(stderr, "       %s [host]\n", __progname);
  exit(1);
 }
@@ -94,14 +95,14 @@ main(int argc, char **argv)
  for (i = 1; i < argc; i++) {
  if (strcmp("--", argv[i]) == 0)
  break;
- if (argv[i][0] != '-' || !isdigit((unsigned char)argv[i][1]))
+ if (argv[i][0] != '-' || !isdigit((u_char)argv[i][1]))
  continue;
 
  if (asprintf(&argv[i], "-s%s", &argv[i][1]) == -1)
  errx(1, "speed asprintf");
  }
 
- while ((opt = getopt(argc, argv, "drl:s:")) != -1) {
+ while ((opt = getopt(argc, argv, "drE:l:s:")) != -1) {
  switch (opt) {
  case 'd':
  is_direct = 1;
@@ -111,6 +112,17 @@ main(int argc, char **argv)
  err(1, "pledge");
  restricted = 1;
  break;
+ case 'E':
+ if (optarg[0] == '^' && optarg[2] == 0 &&
+    (u_char)optarg[1] >= 64 &&
+    (u_char)optarg[1] < 128)
+ escape_char = (u_char)optarg[1] & 31;
+ else if (strlen(optarg) == 1)
+ escape_char = (u_char)optarg[0];
+ else
+ errx(1, "invalid escape character: %s",
+    optarg);
+ break;
  case 'l':
  line_path = optarg;
  break;
@@ -308,14 +320,14 @@ stream_read(struct bufferevent *bufev, void *data)
  last_state = STATE_NEWLINE;
  break;
  case STATE_NEWLINE:
- if (state_change && *ptr == '~') {
- last_state = STATE_TILDE;
+ if (state_change && *ptr == escape_char) {
+ last_state = STATE_ESCAPE;
  continue;
  }
  if (*ptr != '\r')
  last_state = STATE_NONE;
  break;
- case STATE_TILDE:
+ case STATE_ESCAPE:
  do_command(*ptr);
  last_state = STATE_NEWLINE;
  continue;
diff --git a/usr.bin/cu/cu.h b/usr.bin/cu/cu.h
index 2a7ca45d414..abcedc6d77a 100644
--- a/usr.bin/cu/cu.h
+++ b/usr.bin/cu/cu.h
@@ -23,6 +23,7 @@
 void do_command(char);
 
 /* cu.c */
+extern int escape_char;
 extern int restricted;
 extern FILE *record_file;
 extern struct termios saved_tio;

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Nicholas Marriott-2
.
Another couple of minor changes below, with those it looks good to
me. Any OK for this?


On Fri, Mar 15, 2019 at 01:49:52PM +0200, Artturi Alm wrote:

> On Thu, Mar 14, 2019 at 10:18:57AM +0000, Nicholas Marriott wrote:
> > Thanks, comments inline.
> >
>
> The diff looks much better to me, now with those things fixed based
> on your feedback, thanks :]
>
> -Artturi
>
>
> diff --git a/usr.bin/cu/command.c b/usr.bin/cu/command.c
> index c07fe73aeca..27d80f16dd7 100644
> --- a/usr.bin/cu/command.c
> +++ b/usr.bin/cu/command.c
> @@ -30,6 +30,7 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include <vis.h>
>  
>  #include "cu.h"
>  
> @@ -223,6 +224,8 @@ start_record(void)
>  void
>  do_command(char c)
>  {
> + char esc[4 + 1];
> +
>   if (restricted && strchr("CRX$>", c) != NULL) {
>   cu_warnx("~%c command is not allowed in restricted mode", c);
>   return;
> @@ -266,20 +269,23 @@ do_command(char c)
>   sleep(1);
>   ioctl(line_fd, TIOCCBRK, NULL);
>   break;
> - case '~':
> - bufferevent_write(line_ev, "~", 1);
> + default:
> + if (c == escape_char)
> + bufferevent_write(line_ev, &c, 1);
>   break;
>   case '?':
> + vis(esc, escape_char, VIS_WHITE | VIS_NOSLASH, 0);
>   printf("\r\n"
> -    "~#      send break\r\n"
> -    "~$      pipe local command to remote host\r\n"
> -    "~>      send file to remote host\r\n"
> -    "~C      connect program to remote host\r\n"
> -    "~D      de-assert DTR line briefly\r\n"
> -    "~R      start recording to file\r\n"
> -    "~S      set speed\r\n"
> -    "~X      send file with XMODEM\r\n"
> -    "~?      get this summary\r\n"
> +    "%s#      send break\r\n"
> +    "%s$      pipe local command to remote host\r\n"
> +    "%s>      send file to remote host\r\n"
> +    "%sC      connect program to remote host\r\n"
> +    "%sD      de-assert DTR line briefly\r\n"
> +    "%sR      start recording to file\r\n"
> +    "%sS      set speed\r\n"
> +    "%sX      send file with XMODEM\r\n"
> +    "%s?      get this summary\r\n",
> +    esc, esc, esc, esc, esc, esc, esc, esc, esc
>   );
>   break;
>   }
> diff --git a/usr.bin/cu/cu.1 b/usr.bin/cu/cu.1
> index 104a6ea7893..3e85488e87c 100644
> --- a/usr.bin/cu/cu.1
> +++ b/usr.bin/cu/cu.1
> @@ -36,6 +36,7 @@
>  .Sh SYNOPSIS
>  .Nm
>  .Op Fl dr
> +.Op Fl E Ar escape_char
>  .Op Fl l Ar line
>  .Op Fl s Ar speed | Fl Ar speed
>  .Nm
> @@ -55,6 +56,8 @@ The options are as follows:
>  Specify that the line is directly connected and
>  .Nm
>  should not allow the driver to block waiting for a carrier to be detected.
> +.It Fl E Ar escape_char
> +Specify an escape character to use instead of the default tilde.
>  .It Fl l Ar line
>  Specify the line to use.
>  Either of the forms like
> diff --git a/usr.bin/cu/cu.c b/usr.bin/cu/cu.c
> index 03a2df4181f..8e3ce0b0c0e 100644
> --- a/usr.bin/cu/cu.c
> +++ b/usr.bin/cu/cu.c
> @@ -41,6 +41,7 @@ FILE *record_file;
>  struct termios saved_tio;
>  struct bufferevent *input_ev;
>  struct bufferevent *output_ev;
> +int escape_char = '~';
>  int is_direct = -1;
>  int restricted = 0;
>  const char *line_path = NULL;
> @@ -53,7 +54,7 @@ struct event sighup_ev;
>  enum {
>   STATE_NONE,
>   STATE_NEWLINE,
> - STATE_TILDE
> + STATE_ESCAPE
>  } last_state = STATE_NEWLINE;
>  
>  __dead void usage(void);
> @@ -67,8 +68,8 @@ void try_remote(const char *, const char *, const char *);
>  __dead void
>  usage(void)
>  {
> - fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]\n",
> -    __progname);
> + fprintf(stderr, "usage: %s [-dr] [-E escape_char] [-l line] "
> +    "[-s speed | -speed]\n", __progname);
>   fprintf(stderr, "       %s [host]\n", __progname);
>   exit(1);
>  }
> @@ -94,14 +95,14 @@ main(int argc, char **argv)
>   for (i = 1; i < argc; i++) {
>   if (strcmp("--", argv[i]) == 0)
>   break;
> - if (argv[i][0] != '-' || !isdigit((unsigned char)argv[i][1]))
> + if (argv[i][0] != '-' || !isdigit((u_char)argv[i][1]))
>   continue;
>  
>   if (asprintf(&argv[i], "-s%s", &argv[i][1]) == -1)
>   errx(1, "speed asprintf");
>   }
>  
> - while ((opt = getopt(argc, argv, "drl:s:")) != -1) {
> + while ((opt = getopt(argc, argv, "drE:l:s:")) != -1) {
>   switch (opt) {
>   case 'd':
>   is_direct = 1;
> @@ -111,6 +112,17 @@ main(int argc, char **argv)
>   err(1, "pledge");
>   restricted = 1;
>   break;
> + case 'E':
> + if (optarg[0] == '^' && optarg[2] == 0 &&
> +    (u_char)optarg[1] >= 64 &&
> +    (u_char)optarg[1] < 128)

This line can be joined to the previous line now.

> + escape_char = (u_char)optarg[1] & 31;
> + else if (strlen(optarg) == 1)
> + escape_char = (u_char)optarg[0];
> + else
> + errx(1, "invalid escape character: %s",
> +    optarg);

And here as well.

> + break;
>   case 'l':
>   line_path = optarg;
>   break;
> @@ -308,14 +320,14 @@ stream_read(struct bufferevent *bufev, void *data)
>   last_state = STATE_NEWLINE;
>   break;
>   case STATE_NEWLINE:
> - if (state_change && *ptr == '~') {
> - last_state = STATE_TILDE;
> + if (state_change && *ptr == escape_char) {

This needs to be (u_char)*ptr.

> + last_state = STATE_ESCAPE;
>   continue;
>   }
>   if (*ptr != '\r')
>   last_state = STATE_NONE;
>   break;
> - case STATE_TILDE:
> + case STATE_ESCAPE:
>   do_command(*ptr);
>   last_state = STATE_NEWLINE;
>   continue;
> diff --git a/usr.bin/cu/cu.h b/usr.bin/cu/cu.h
> index 2a7ca45d414..abcedc6d77a 100644
> --- a/usr.bin/cu/cu.h
> +++ b/usr.bin/cu/cu.h
> @@ -23,6 +23,7 @@
>  void do_command(char);
>  
>  /* cu.c */
> +extern int escape_char;
>  extern int restricted;
>  extern FILE *record_file;
>  extern struct termios saved_tio;

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Hiltjo Posthuma
On Fri, Mar 15, 2019 at 02:43:04PM +0000, Nicholas Marriott wrote:

> .
> Another couple of minor changes below, with those it looks good to
> me. Any OK for this?
>
>
> On Fri, Mar 15, 2019 at 01:49:52PM +0200, Artturi Alm wrote:
> > On Thu, Mar 14, 2019 at 10:18:57AM +0000, Nicholas Marriott wrote:
> > > Thanks, comments inline.
> > >
> >
> > The diff looks much better to me, now with those things fixed based
> > on your feedback, thanks :]
> >
> > -Artturi
> >
> >
> > diff --git a/usr.bin/cu/command.c b/usr.bin/cu/command.c
> > index c07fe73aeca..27d80f16dd7 100644
> > --- a/usr.bin/cu/command.c
> > +++ b/usr.bin/cu/command.c
> > @@ -30,6 +30,7 @@
> >  #include <stdio.h>
> >  #include <string.h>
> >  #include <unistd.h>
> > +#include <vis.h>
> >  
> >  #include "cu.h"
> >  
> > @@ -223,6 +224,8 @@ start_record(void)
> >  void
> >  do_command(char c)
> >  {
> > + char esc[4 + 1];
> > +
> >   if (restricted && strchr("CRX$>", c) != NULL) {
> >   cu_warnx("~%c command is not allowed in restricted mode", c);
> >   return;
> > @@ -266,20 +269,23 @@ do_command(char c)
> >   sleep(1);
> >   ioctl(line_fd, TIOCCBRK, NULL);
> >   break;
> > - case '~':
> > - bufferevent_write(line_ev, "~", 1);
> > + default:
> > + if (c == escape_char)
> > + bufferevent_write(line_ev, &c, 1);
> >   break;
> >   case '?':
> > + vis(esc, escape_char, VIS_WHITE | VIS_NOSLASH, 0);
> >   printf("\r\n"
> > -    "~#      send break\r\n"
> > -    "~$      pipe local command to remote host\r\n"
> > -    "~>      send file to remote host\r\n"
> > -    "~C      connect program to remote host\r\n"
> > -    "~D      de-assert DTR line briefly\r\n"
> > -    "~R      start recording to file\r\n"
> > -    "~S      set speed\r\n"
> > -    "~X      send file with XMODEM\r\n"
> > -    "~?      get this summary\r\n"
> > +    "%s#      send break\r\n"
> > +    "%s$      pipe local command to remote host\r\n"
> > +    "%s>      send file to remote host\r\n"
> > +    "%sC      connect program to remote host\r\n"
> > +    "%sD      de-assert DTR line briefly\r\n"
> > +    "%sR      start recording to file\r\n"
> > +    "%sS      set speed\r\n"
> > +    "%sX      send file with XMODEM\r\n"
> > +    "%s?      get this summary\r\n",
> > +    esc, esc, esc, esc, esc, esc, esc, esc, esc
> >   );

Maybe it's nicer to use numbered arguments for this instead of repeating esc?
Like: printf("%1$s %1$s %1$s", esc);


> >   break;
> >   }
> > diff --git a/usr.bin/cu/cu.1 b/usr.bin/cu/cu.1
> > index 104a6ea7893..3e85488e87c 100644
> > --- a/usr.bin/cu/cu.1
> > +++ b/usr.bin/cu/cu.1
> > @@ -36,6 +36,7 @@
> >  .Sh SYNOPSIS
> >  .Nm
> >  .Op Fl dr
> > +.Op Fl E Ar escape_char
> >  .Op Fl l Ar line
> >  .Op Fl s Ar speed | Fl Ar speed
> >  .Nm
> > @@ -55,6 +56,8 @@ The options are as follows:
> >  Specify that the line is directly connected and
> >  .Nm
> >  should not allow the driver to block waiting for a carrier to be detected.
> > +.It Fl E Ar escape_char
> > +Specify an escape character to use instead of the default tilde.
> >  .It Fl l Ar line
> >  Specify the line to use.
> >  Either of the forms like
> > diff --git a/usr.bin/cu/cu.c b/usr.bin/cu/cu.c
> > index 03a2df4181f..8e3ce0b0c0e 100644
> > --- a/usr.bin/cu/cu.c
> > +++ b/usr.bin/cu/cu.c
> > @@ -41,6 +41,7 @@ FILE *record_file;
> >  struct termios saved_tio;
> >  struct bufferevent *input_ev;
> >  struct bufferevent *output_ev;
> > +int escape_char = '~';
> >  int is_direct = -1;
> >  int restricted = 0;
> >  const char *line_path = NULL;
> > @@ -53,7 +54,7 @@ struct event sighup_ev;
> >  enum {
> >   STATE_NONE,
> >   STATE_NEWLINE,
> > - STATE_TILDE
> > + STATE_ESCAPE
> >  } last_state = STATE_NEWLINE;
> >  
> >  __dead void usage(void);
> > @@ -67,8 +68,8 @@ void try_remote(const char *, const char *, const char *);
> >  __dead void
> >  usage(void)
> >  {
> > - fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]\n",
> > -    __progname);
> > + fprintf(stderr, "usage: %s [-dr] [-E escape_char] [-l line] "
> > +    "[-s speed | -speed]\n", __progname);
> >   fprintf(stderr, "       %s [host]\n", __progname);
> >   exit(1);
> >  }
> > @@ -94,14 +95,14 @@ main(int argc, char **argv)
> >   for (i = 1; i < argc; i++) {
> >   if (strcmp("--", argv[i]) == 0)
> >   break;
> > - if (argv[i][0] != '-' || !isdigit((unsigned char)argv[i][1]))
> > + if (argv[i][0] != '-' || !isdigit((u_char)argv[i][1]))
> >   continue;
> >  
> >   if (asprintf(&argv[i], "-s%s", &argv[i][1]) == -1)
> >   errx(1, "speed asprintf");
> >   }
> >  
> > - while ((opt = getopt(argc, argv, "drl:s:")) != -1) {
> > + while ((opt = getopt(argc, argv, "drE:l:s:")) != -1) {
> >   switch (opt) {
> >   case 'd':
> >   is_direct = 1;
> > @@ -111,6 +112,17 @@ main(int argc, char **argv)
> >   err(1, "pledge");
> >   restricted = 1;
> >   break;
> > + case 'E':
> > + if (optarg[0] == '^' && optarg[2] == 0 &&
> > +    (u_char)optarg[1] >= 64 &&
> > +    (u_char)optarg[1] < 128)
>
> This line can be joined to the previous line now.
>
> > + escape_char = (u_char)optarg[1] & 31;
> > + else if (strlen(optarg) == 1)
> > + escape_char = (u_char)optarg[0];
> > + else
> > + errx(1, "invalid escape character: %s",
> > +    optarg);
>
> And here as well.
>
> > + break;
> >   case 'l':
> >   line_path = optarg;
> >   break;
> > @@ -308,14 +320,14 @@ stream_read(struct bufferevent *bufev, void *data)
> >   last_state = STATE_NEWLINE;
> >   break;
> >   case STATE_NEWLINE:
> > - if (state_change && *ptr == '~') {
> > - last_state = STATE_TILDE;
> > + if (state_change && *ptr == escape_char) {
>
> This needs to be (u_char)*ptr.
>
> > + last_state = STATE_ESCAPE;
> >   continue;
> >   }
> >   if (*ptr != '\r')
> >   last_state = STATE_NONE;
> >   break;
> > - case STATE_TILDE:
> > + case STATE_ESCAPE:
> >   do_command(*ptr);
> >   last_state = STATE_NEWLINE;
> >   continue;
> > diff --git a/usr.bin/cu/cu.h b/usr.bin/cu/cu.h
> > index 2a7ca45d414..abcedc6d77a 100644
> > --- a/usr.bin/cu/cu.h
> > +++ b/usr.bin/cu/cu.h
> > @@ -23,6 +23,7 @@
> >  void do_command(char);
> >  
> >  /* cu.c */
> > +extern int escape_char;
> >  extern int restricted;
> >  extern FILE *record_file;
> >  extern struct termios saved_tio;
>

--
Kind regards,
Hiltjo

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Todd C. Miller-3
In reply to this post by Artturi Alm
Wouldn't it be less error-prone to make escape_char u_char instead
of int?

 - todd

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Artturi Alm
In reply to this post by Nicholas Marriott-2
On Fri, Mar 15, 2019 at 02:43:04PM +0000, Nicholas Marriott wrote:
> .
> Another couple of minor changes below, with those it looks good to
> me. Any OK for this?
>

With joined lines, the cast, and some runtime testing with ~, % and ^[.

-Artturi

diff --git a/usr.bin/cu/command.c b/usr.bin/cu/command.c
index c07fe73aeca..27d80f16dd7 100644
--- a/usr.bin/cu/command.c
+++ b/usr.bin/cu/command.c
@@ -30,6 +30,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <vis.h>
 
 #include "cu.h"
 
@@ -223,6 +224,8 @@ start_record(void)
 void
 do_command(char c)
 {
+ char esc[4 + 1];
+
  if (restricted && strchr("CRX$>", c) != NULL) {
  cu_warnx("~%c command is not allowed in restricted mode", c);
  return;
@@ -266,20 +269,23 @@ do_command(char c)
  sleep(1);
  ioctl(line_fd, TIOCCBRK, NULL);
  break;
- case '~':
- bufferevent_write(line_ev, "~", 1);
+ default:
+ if (c == escape_char)
+ bufferevent_write(line_ev, &c, 1);
  break;
  case '?':
+ vis(esc, escape_char, VIS_WHITE | VIS_NOSLASH, 0);
  printf("\r\n"
-    "~#      send break\r\n"
-    "~$      pipe local command to remote host\r\n"
-    "~>      send file to remote host\r\n"
-    "~C      connect program to remote host\r\n"
-    "~D      de-assert DTR line briefly\r\n"
-    "~R      start recording to file\r\n"
-    "~S      set speed\r\n"
-    "~X      send file with XMODEM\r\n"
-    "~?      get this summary\r\n"
+    "%s#      send break\r\n"
+    "%s$      pipe local command to remote host\r\n"
+    "%s>      send file to remote host\r\n"
+    "%sC      connect program to remote host\r\n"
+    "%sD      de-assert DTR line briefly\r\n"
+    "%sR      start recording to file\r\n"
+    "%sS      set speed\r\n"
+    "%sX      send file with XMODEM\r\n"
+    "%s?      get this summary\r\n",
+    esc, esc, esc, esc, esc, esc, esc, esc, esc
  );
  break;
  }
diff --git a/usr.bin/cu/cu.1 b/usr.bin/cu/cu.1
index 104a6ea7893..3e85488e87c 100644
--- a/usr.bin/cu/cu.1
+++ b/usr.bin/cu/cu.1
@@ -36,6 +36,7 @@
 .Sh SYNOPSIS
 .Nm
 .Op Fl dr
+.Op Fl E Ar escape_char
 .Op Fl l Ar line
 .Op Fl s Ar speed | Fl Ar speed
 .Nm
@@ -55,6 +56,8 @@ The options are as follows:
 Specify that the line is directly connected and
 .Nm
 should not allow the driver to block waiting for a carrier to be detected.
+.It Fl E Ar escape_char
+Specify an escape character to use instead of the default tilde.
 .It Fl l Ar line
 Specify the line to use.
 Either of the forms like
diff --git a/usr.bin/cu/cu.c b/usr.bin/cu/cu.c
index 03a2df4181f..d01c3327042 100644
--- a/usr.bin/cu/cu.c
+++ b/usr.bin/cu/cu.c
@@ -41,6 +41,7 @@ FILE *record_file;
 struct termios saved_tio;
 struct bufferevent *input_ev;
 struct bufferevent *output_ev;
+int escape_char = '~';
 int is_direct = -1;
 int restricted = 0;
 const char *line_path = NULL;
@@ -53,7 +54,7 @@ struct event sighup_ev;
 enum {
  STATE_NONE,
  STATE_NEWLINE,
- STATE_TILDE
+ STATE_ESCAPE
 } last_state = STATE_NEWLINE;
 
 __dead void usage(void);
@@ -67,8 +68,8 @@ void try_remote(const char *, const char *, const char *);
 __dead void
 usage(void)
 {
- fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]\n",
-    __progname);
+ fprintf(stderr, "usage: %s [-dr] [-E escape_char] [-l line] "
+    "[-s speed | -speed]\n", __progname);
  fprintf(stderr, "       %s [host]\n", __progname);
  exit(1);
 }
@@ -94,14 +95,14 @@ main(int argc, char **argv)
  for (i = 1; i < argc; i++) {
  if (strcmp("--", argv[i]) == 0)
  break;
- if (argv[i][0] != '-' || !isdigit((unsigned char)argv[i][1]))
+ if (argv[i][0] != '-' || !isdigit((u_char)argv[i][1]))
  continue;
 
  if (asprintf(&argv[i], "-s%s", &argv[i][1]) == -1)
  errx(1, "speed asprintf");
  }
 
- while ((opt = getopt(argc, argv, "drl:s:")) != -1) {
+ while ((opt = getopt(argc, argv, "drE:l:s:")) != -1) {
  switch (opt) {
  case 'd':
  is_direct = 1;
@@ -111,6 +112,15 @@ main(int argc, char **argv)
  err(1, "pledge");
  restricted = 1;
  break;
+ case 'E':
+ if (optarg[0] == '^' && optarg[2] == 0 &&
+    (u_char)optarg[1] >= 64 && (u_char)optarg[1] < 128)
+ escape_char = (u_char)optarg[1] & 31;
+ else if (strlen(optarg) == 1)
+ escape_char = (u_char)optarg[0];
+ else
+ errx(1, "invalid escape character: %s", optarg);
+ break;
  case 'l':
  line_path = optarg;
  break;
@@ -308,14 +318,14 @@ stream_read(struct bufferevent *bufev, void *data)
  last_state = STATE_NEWLINE;
  break;
  case STATE_NEWLINE:
- if (state_change && *ptr == '~') {
- last_state = STATE_TILDE;
+ if (state_change && (u_char)*ptr == escape_char) {
+ last_state = STATE_ESCAPE;
  continue;
  }
  if (*ptr != '\r')
  last_state = STATE_NONE;
  break;
- case STATE_TILDE:
+ case STATE_ESCAPE:
  do_command(*ptr);
  last_state = STATE_NEWLINE;
  continue;
diff --git a/usr.bin/cu/cu.h b/usr.bin/cu/cu.h
index 2a7ca45d414..abcedc6d77a 100644
--- a/usr.bin/cu/cu.h
+++ b/usr.bin/cu/cu.h
@@ -23,6 +23,7 @@
 void do_command(char);
 
 /* cu.c */
+extern int escape_char;
 extern int restricted;
 extern FILE *record_file;
 extern struct termios saved_tio;

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Nicholas Marriott-2
In reply to this post by Todd C. Miller-3
On Fri, Mar 15, 2019 at 09:43:56AM -0600, Todd C. Miller wrote:
> Wouldn't it be less error-prone to make escape_char u_char instead
> of int?

Maybe, I don't mind either way.

However this in stream_read would still need a cast as ptr is signed:

        if (state_change && (u_char)*ptr == escape_char) {

Although there is no reason ptr couldn't be u_char * too (in that case
it would be nice to change line_read as well).

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Artturi Alm
On Fri, Mar 15, 2019 at 09:51:12PM +0000, Nicholas Marriott wrote:

> On Fri, Mar 15, 2019 at 09:43:56AM -0600, Todd C. Miller wrote:
> > Wouldn't it be less error-prone to make escape_char u_char instead
> > of int?
>
> Maybe, I don't mind either way.
>
> However this in stream_read would still need a cast as ptr is signed:
>
> if (state_change && (u_char)*ptr == escape_char) {
>
> Although there is no reason ptr couldn't be u_char * too (in that case
> it would be nice to change line_read as well).

do_command(char c) was why I didn't go for u_char, and kept it int as
it was in ssh.

I've done some builds of cu with -Wpointer-sign etc., but the output
didn't seem anything like i would like to minimally solve, or cause to
anyone else. :]

-Artturi

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Nicholas Marriott-2
In reply to this post by Artturi Alm

This is OK with me, anyone else?


On Fri, Mar 15, 2019 at 06:56:31PM +0200, Artturi Alm wrote:

> On Fri, Mar 15, 2019 at 02:43:04PM +0000, Nicholas Marriott wrote:
> > .
> > Another couple of minor changes below, with those it looks good to
> > me. Any OK for this?
> >
>
> With joined lines, the cast, and some runtime testing with ~, % and ^[.
>
> -Artturi
>
> diff --git a/usr.bin/cu/command.c b/usr.bin/cu/command.c
> index c07fe73aeca..27d80f16dd7 100644
> --- a/usr.bin/cu/command.c
> +++ b/usr.bin/cu/command.c
> @@ -30,6 +30,7 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <unistd.h>
> +#include <vis.h>
>  
>  #include "cu.h"
>  
> @@ -223,6 +224,8 @@ start_record(void)
>  void
>  do_command(char c)
>  {
> + char esc[4 + 1];
> +
>   if (restricted && strchr("CRX$>", c) != NULL) {
>   cu_warnx("~%c command is not allowed in restricted mode", c);
>   return;
> @@ -266,20 +269,23 @@ do_command(char c)
>   sleep(1);
>   ioctl(line_fd, TIOCCBRK, NULL);
>   break;
> - case '~':
> - bufferevent_write(line_ev, "~", 1);
> + default:
> + if (c == escape_char)
> + bufferevent_write(line_ev, &c, 1);
>   break;
>   case '?':
> + vis(esc, escape_char, VIS_WHITE | VIS_NOSLASH, 0);
>   printf("\r\n"
> -    "~#      send break\r\n"
> -    "~$      pipe local command to remote host\r\n"
> -    "~>      send file to remote host\r\n"
> -    "~C      connect program to remote host\r\n"
> -    "~D      de-assert DTR line briefly\r\n"
> -    "~R      start recording to file\r\n"
> -    "~S      set speed\r\n"
> -    "~X      send file with XMODEM\r\n"
> -    "~?      get this summary\r\n"
> +    "%s#      send break\r\n"
> +    "%s$      pipe local command to remote host\r\n"
> +    "%s>      send file to remote host\r\n"
> +    "%sC      connect program to remote host\r\n"
> +    "%sD      de-assert DTR line briefly\r\n"
> +    "%sR      start recording to file\r\n"
> +    "%sS      set speed\r\n"
> +    "%sX      send file with XMODEM\r\n"
> +    "%s?      get this summary\r\n",
> +    esc, esc, esc, esc, esc, esc, esc, esc, esc
>   );
>   break;
>   }
> diff --git a/usr.bin/cu/cu.1 b/usr.bin/cu/cu.1
> index 104a6ea7893..3e85488e87c 100644
> --- a/usr.bin/cu/cu.1
> +++ b/usr.bin/cu/cu.1
> @@ -36,6 +36,7 @@
>  .Sh SYNOPSIS
>  .Nm
>  .Op Fl dr
> +.Op Fl E Ar escape_char
>  .Op Fl l Ar line
>  .Op Fl s Ar speed | Fl Ar speed
>  .Nm
> @@ -55,6 +56,8 @@ The options are as follows:
>  Specify that the line is directly connected and
>  .Nm
>  should not allow the driver to block waiting for a carrier to be detected.
> +.It Fl E Ar escape_char
> +Specify an escape character to use instead of the default tilde.
>  .It Fl l Ar line
>  Specify the line to use.
>  Either of the forms like
> diff --git a/usr.bin/cu/cu.c b/usr.bin/cu/cu.c
> index 03a2df4181f..d01c3327042 100644
> --- a/usr.bin/cu/cu.c
> +++ b/usr.bin/cu/cu.c
> @@ -41,6 +41,7 @@ FILE *record_file;
>  struct termios saved_tio;
>  struct bufferevent *input_ev;
>  struct bufferevent *output_ev;
> +int escape_char = '~';
>  int is_direct = -1;
>  int restricted = 0;
>  const char *line_path = NULL;
> @@ -53,7 +54,7 @@ struct event sighup_ev;
>  enum {
>   STATE_NONE,
>   STATE_NEWLINE,
> - STATE_TILDE
> + STATE_ESCAPE
>  } last_state = STATE_NEWLINE;
>  
>  __dead void usage(void);
> @@ -67,8 +68,8 @@ void try_remote(const char *, const char *, const char *);
>  __dead void
>  usage(void)
>  {
> - fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]\n",
> -    __progname);
> + fprintf(stderr, "usage: %s [-dr] [-E escape_char] [-l line] "
> +    "[-s speed | -speed]\n", __progname);
>   fprintf(stderr, "       %s [host]\n", __progname);
>   exit(1);
>  }
> @@ -94,14 +95,14 @@ main(int argc, char **argv)
>   for (i = 1; i < argc; i++) {
>   if (strcmp("--", argv[i]) == 0)
>   break;
> - if (argv[i][0] != '-' || !isdigit((unsigned char)argv[i][1]))
> + if (argv[i][0] != '-' || !isdigit((u_char)argv[i][1]))
>   continue;
>  
>   if (asprintf(&argv[i], "-s%s", &argv[i][1]) == -1)
>   errx(1, "speed asprintf");
>   }
>  
> - while ((opt = getopt(argc, argv, "drl:s:")) != -1) {
> + while ((opt = getopt(argc, argv, "drE:l:s:")) != -1) {
>   switch (opt) {
>   case 'd':
>   is_direct = 1;
> @@ -111,6 +112,15 @@ main(int argc, char **argv)
>   err(1, "pledge");
>   restricted = 1;
>   break;
> + case 'E':
> + if (optarg[0] == '^' && optarg[2] == 0 &&
> +    (u_char)optarg[1] >= 64 && (u_char)optarg[1] < 128)
> + escape_char = (u_char)optarg[1] & 31;
> + else if (strlen(optarg) == 1)
> + escape_char = (u_char)optarg[0];
> + else
> + errx(1, "invalid escape character: %s", optarg);
> + break;
>   case 'l':
>   line_path = optarg;
>   break;
> @@ -308,14 +318,14 @@ stream_read(struct bufferevent *bufev, void *data)
>   last_state = STATE_NEWLINE;
>   break;
>   case STATE_NEWLINE:
> - if (state_change && *ptr == '~') {
> - last_state = STATE_TILDE;
> + if (state_change && (u_char)*ptr == escape_char) {
> + last_state = STATE_ESCAPE;
>   continue;
>   }
>   if (*ptr != '\r')
>   last_state = STATE_NONE;
>   break;
> - case STATE_TILDE:
> + case STATE_ESCAPE:
>   do_command(*ptr);
>   last_state = STATE_NEWLINE;
>   continue;
> diff --git a/usr.bin/cu/cu.h b/usr.bin/cu/cu.h
> index 2a7ca45d414..abcedc6d77a 100644
> --- a/usr.bin/cu/cu.h
> +++ b/usr.bin/cu/cu.h
> @@ -23,6 +23,7 @@
>  void do_command(char);
>  
>  /* cu.c */
> +extern int escape_char;
>  extern int restricted;
>  extern FILE *record_file;
>  extern struct termios saved_tio;

Reply | Threaded
Open this post in threaded view
|

Re: Option for alternative escape character with cu(1)

Nicholas Marriott-2

I will commit this in the next few days unless I hear objections.



On Mon, Mar 18, 2019 at 07:39:45AM +0000, Nicholas Marriott wrote:

>
> This is OK with me, anyone else?
>
>
> On Fri, Mar 15, 2019 at 06:56:31PM +0200, Artturi Alm wrote:
> > On Fri, Mar 15, 2019 at 02:43:04PM +0000, Nicholas Marriott wrote:
> > > .
> > > Another couple of minor changes below, with those it looks good to
> > > me. Any OK for this?
> > >
> >
> > With joined lines, the cast, and some runtime testing with ~, % and ^[.
> >
> > -Artturi
> >
> > diff --git a/usr.bin/cu/command.c b/usr.bin/cu/command.c
> > index c07fe73aeca..27d80f16dd7 100644
> > --- a/usr.bin/cu/command.c
> > +++ b/usr.bin/cu/command.c
> > @@ -30,6 +30,7 @@
> >  #include <stdio.h>
> >  #include <string.h>
> >  #include <unistd.h>
> > +#include <vis.h>
> >  
> >  #include "cu.h"
> >  
> > @@ -223,6 +224,8 @@ start_record(void)
> >  void
> >  do_command(char c)
> >  {
> > + char esc[4 + 1];
> > +
> >   if (restricted && strchr("CRX$>", c) != NULL) {
> >   cu_warnx("~%c command is not allowed in restricted mode", c);
> >   return;
> > @@ -266,20 +269,23 @@ do_command(char c)
> >   sleep(1);
> >   ioctl(line_fd, TIOCCBRK, NULL);
> >   break;
> > - case '~':
> > - bufferevent_write(line_ev, "~", 1);
> > + default:
> > + if (c == escape_char)
> > + bufferevent_write(line_ev, &c, 1);
> >   break;
> >   case '?':
> > + vis(esc, escape_char, VIS_WHITE | VIS_NOSLASH, 0);
> >   printf("\r\n"
> > -    "~#      send break\r\n"
> > -    "~$      pipe local command to remote host\r\n"
> > -    "~>      send file to remote host\r\n"
> > -    "~C      connect program to remote host\r\n"
> > -    "~D      de-assert DTR line briefly\r\n"
> > -    "~R      start recording to file\r\n"
> > -    "~S      set speed\r\n"
> > -    "~X      send file with XMODEM\r\n"
> > -    "~?      get this summary\r\n"
> > +    "%s#      send break\r\n"
> > +    "%s$      pipe local command to remote host\r\n"
> > +    "%s>      send file to remote host\r\n"
> > +    "%sC      connect program to remote host\r\n"
> > +    "%sD      de-assert DTR line briefly\r\n"
> > +    "%sR      start recording to file\r\n"
> > +    "%sS      set speed\r\n"
> > +    "%sX      send file with XMODEM\r\n"
> > +    "%s?      get this summary\r\n",
> > +    esc, esc, esc, esc, esc, esc, esc, esc, esc
> >   );
> >   break;
> >   }
> > diff --git a/usr.bin/cu/cu.1 b/usr.bin/cu/cu.1
> > index 104a6ea7893..3e85488e87c 100644
> > --- a/usr.bin/cu/cu.1
> > +++ b/usr.bin/cu/cu.1
> > @@ -36,6 +36,7 @@
> >  .Sh SYNOPSIS
> >  .Nm
> >  .Op Fl dr
> > +.Op Fl E Ar escape_char
> >  .Op Fl l Ar line
> >  .Op Fl s Ar speed | Fl Ar speed
> >  .Nm
> > @@ -55,6 +56,8 @@ The options are as follows:
> >  Specify that the line is directly connected and
> >  .Nm
> >  should not allow the driver to block waiting for a carrier to be detected.
> > +.It Fl E Ar escape_char
> > +Specify an escape character to use instead of the default tilde.
> >  .It Fl l Ar line
> >  Specify the line to use.
> >  Either of the forms like
> > diff --git a/usr.bin/cu/cu.c b/usr.bin/cu/cu.c
> > index 03a2df4181f..d01c3327042 100644
> > --- a/usr.bin/cu/cu.c
> > +++ b/usr.bin/cu/cu.c
> > @@ -41,6 +41,7 @@ FILE *record_file;
> >  struct termios saved_tio;
> >  struct bufferevent *input_ev;
> >  struct bufferevent *output_ev;
> > +int escape_char = '~';
> >  int is_direct = -1;
> >  int restricted = 0;
> >  const char *line_path = NULL;
> > @@ -53,7 +54,7 @@ struct event sighup_ev;
> >  enum {
> >   STATE_NONE,
> >   STATE_NEWLINE,
> > - STATE_TILDE
> > + STATE_ESCAPE
> >  } last_state = STATE_NEWLINE;
> >  
> >  __dead void usage(void);
> > @@ -67,8 +68,8 @@ void try_remote(const char *, const char *, const char *);
> >  __dead void
> >  usage(void)
> >  {
> > - fprintf(stderr, "usage: %s [-dr] [-l line] [-s speed | -speed]\n",
> > -    __progname);
> > + fprintf(stderr, "usage: %s [-dr] [-E escape_char] [-l line] "
> > +    "[-s speed | -speed]\n", __progname);
> >   fprintf(stderr, "       %s [host]\n", __progname);
> >   exit(1);
> >  }
> > @@ -94,14 +95,14 @@ main(int argc, char **argv)
> >   for (i = 1; i < argc; i++) {
> >   if (strcmp("--", argv[i]) == 0)
> >   break;
> > - if (argv[i][0] != '-' || !isdigit((unsigned char)argv[i][1]))
> > + if (argv[i][0] != '-' || !isdigit((u_char)argv[i][1]))
> >   continue;
> >  
> >   if (asprintf(&argv[i], "-s%s", &argv[i][1]) == -1)
> >   errx(1, "speed asprintf");
> >   }
> >  
> > - while ((opt = getopt(argc, argv, "drl:s:")) != -1) {
> > + while ((opt = getopt(argc, argv, "drE:l:s:")) != -1) {
> >   switch (opt) {
> >   case 'd':
> >   is_direct = 1;
> > @@ -111,6 +112,15 @@ main(int argc, char **argv)
> >   err(1, "pledge");
> >   restricted = 1;
> >   break;
> > + case 'E':
> > + if (optarg[0] == '^' && optarg[2] == 0 &&
> > +    (u_char)optarg[1] >= 64 && (u_char)optarg[1] < 128)
> > + escape_char = (u_char)optarg[1] & 31;
> > + else if (strlen(optarg) == 1)
> > + escape_char = (u_char)optarg[0];
> > + else
> > + errx(1, "invalid escape character: %s", optarg);
> > + break;
> >   case 'l':
> >   line_path = optarg;
> >   break;
> > @@ -308,14 +318,14 @@ stream_read(struct bufferevent *bufev, void *data)
> >   last_state = STATE_NEWLINE;
> >   break;
> >   case STATE_NEWLINE:
> > - if (state_change && *ptr == '~') {
> > - last_state = STATE_TILDE;
> > + if (state_change && (u_char)*ptr == escape_char) {
> > + last_state = STATE_ESCAPE;
> >   continue;
> >   }
> >   if (*ptr != '\r')
> >   last_state = STATE_NONE;
> >   break;
> > - case STATE_TILDE:
> > + case STATE_ESCAPE:
> >   do_command(*ptr);
> >   last_state = STATE_NEWLINE;
> >   continue;
> > diff --git a/usr.bin/cu/cu.h b/usr.bin/cu/cu.h
> > index 2a7ca45d414..abcedc6d77a 100644
> > --- a/usr.bin/cu/cu.h
> > +++ b/usr.bin/cu/cu.h
> > @@ -23,6 +23,7 @@
> >  void do_command(char);
> >  
> >  /* cu.c */
> > +extern int escape_char;
> >  extern int restricted;
> >  extern FILE *record_file;
> >  extern struct termios saved_tio;