[systemd-devel] [PATCH 06/11] There is no ANSI support on common 3215 consoles

Zbigniew Jędrzejewski-Szmek zbyszek at in.waw.pl
Thu Jun 19 20:06:06 PDT 2014


On Fri, Jun 13, 2014 at 04:41:05PM +0200, Werner Fink wrote:
> Therefore strip off the ANSI escape sequences for 3215 consoles
> but support 3270 consoles if found.
> 
> ---
>  src/core/manager.c |   24 ++++++++++++----
>  src/shared/util.c  |   80 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  src/shared/util.h  |    1 +
>  3 files changed, 96 insertions(+), 9 deletions(-)
> 
> diff --git src/core/manager.c src/core/manager.c
> index 0cb2044..f78ac18 100644
> --- src/core/manager.c
> +++ src/core/manager.c
> @@ -112,7 +112,7 @@ static int manager_watch_jobs_in_progress(Manager *m) {
>  
>  #define CYLON_BUFFER_EXTRA (2*(sizeof(ANSI_RED_ON)-1) + sizeof(ANSI_HIGHLIGHT_RED_ON)-1 + 2*(sizeof(ANSI_HIGHLIGHT_OFF)-1))

The goal of this patch seems correct. Nevertheless, it is quite
intrusive. Could you rework the patch to:

1. centralize the ansi console detection, like we do with on_tty(),
   so it is implicitly checked on first access and then cached
   inside of the function;
2. replace ANSI_RED_ON/OFF/... with ansi_red_on(), which calls the ansi console
   status function and returns either ANSI_RED_ON/OFF/... or "";
3. instead of parsing the message to extract status in the status_printf
   function, pass the status as a (numerical) argument, and use this
   to decide the output. The mapping should be stored in a table;
4. use parse_proc_cmdline to parse /proc/cmdline.

Thanks,
Zbyszek

>  
> -static void draw_cylon(char buffer[], size_t buflen, unsigned width, unsigned pos) {
> +static void draw_cylon(char buffer[], size_t buflen, unsigned width, unsigned pos, bool ansi_console) {
>          char *p = buffer;
>  
>          assert(buflen >= CYLON_BUFFER_EXTRA + width + 1);
> @@ -121,12 +121,14 @@ static void draw_cylon(char buffer[], size_t buflen, unsigned width, unsigned po
>          if (pos > 1) {
>                  if (pos > 2)
>                          p = mempset(p, ' ', pos-2);
> -                p = stpcpy(p, ANSI_RED_ON);
> +                if (ansi_console)
> +                        p = stpcpy(p, ANSI_RED_ON);
>                  *p++ = '*';
>          }
>  
>          if (pos > 0 && pos <= width) {
> -                p = stpcpy(p, ANSI_HIGHLIGHT_RED_ON);
> +                if (ansi_console)
> +                        p = stpcpy(p, ANSI_HIGHLIGHT_RED_ON);
>                  *p++ = '*';
>          }
>  
> @@ -137,7 +139,8 @@ static void draw_cylon(char buffer[], size_t buflen, unsigned width, unsigned po
>                  *p++ = '*';
>                  if (pos < width-1)
>                          p = mempset(p, ' ', width-1-pos);
> -                strcpy(p, ANSI_HIGHLIGHT_OFF);
> +                if (ansi_console)
> +                        strcpy(p, ANSI_HIGHLIGHT_OFF);
>          }
>  }
>  
> @@ -154,6 +157,7 @@ void manager_flip_auto_status(Manager *m, bool enable) {
>  }
>  
>  static void manager_print_jobs_in_progress(Manager *m) {
> +        static int is_ansi_console = -1;
>          _cleanup_free_ char *job_of_n = NULL;
>          Iterator i;
>          Job *j;
> @@ -178,10 +182,20 @@ static void manager_print_jobs_in_progress(Manager *m) {
>          assert(counter == print_nr + 1);
>          assert(j);
>  
> +        if (_unlikely_(is_ansi_console < 0)) {
> +                int fd = open_terminal("/dev/console", O_RDONLY|O_NOCTTY|O_CLOEXEC);
> +                if (fd < 0)
> +                        is_ansi_console = 0;
> +                else {
> +                        is_ansi_console = (int)ansi_console(fd);
> +                        close(fd);
> +                }
> +        }
> +
>          cylon_pos = m->jobs_in_progress_iteration % 14;
>          if (cylon_pos >= 8)
>                  cylon_pos = 14 - cylon_pos;
> -        draw_cylon(cylon, sizeof(cylon), 6, cylon_pos);
> +        draw_cylon(cylon, sizeof(cylon), 6, cylon_pos, (bool)is_ansi_console);
>  
>          m->jobs_in_progress_iteration++;
>  
> diff --git src/shared/util.c src/shared/util.c
> index a7aec5c..03b01de 100644
> --- src/shared/util.c
> +++ src/shared/util.c
> @@ -2937,6 +2937,7 @@ int status_vprintf(const char *status, bool ellipse, bool ephemeral, const char
>          struct iovec iovec[6] = {};
>          int n = 0;
>          static bool prev_ephemeral;
> +        static int is_ansi_console = -1;
>  
>          assert(format);
>  
> @@ -2950,6 +2951,41 @@ int status_vprintf(const char *status, bool ellipse, bool ephemeral, const char
>          if (fd < 0)
>                  return fd;
>  
> +        if (_unlikely_(is_ansi_console < 0))
> +                is_ansi_console = (int)ansi_console(fd);
> +
> +        if (status && !is_ansi_console) {
> +                const char *esc, *ptr;
> +                esc = strchr(status, 0x1B);
> +                if (esc && (ptr = strpbrk(esc, "SOFDTI*"))) {
> +                        switch(*ptr) {
> +                        case 'S':
> +                                status = " SKIP ";
> +                                break;
> +                        case 'O':
> +                                status = "  OK  ";
> +                                break;
> +                        case 'F':
> +                                status = "FAILED";
> +                                break;
> +                        case 'D':
> +                                status = "DEPEND";
> +                                break;
> +                        case 'T':
> +                                status = " TIME ";
> +                                break;
> +                        case 'I':
> +                                status = " INFO ";
> +                                break;
> +                        case '*':
> +                                status = " BUSY ";
> +                                break;
> +                        default:
> +                                break;
> +                        }
> +                }
> +        }
> +
>          if (ellipse) {
>                  char *e;
>                  size_t emax, sl;
> @@ -2972,8 +3008,12 @@ int status_vprintf(const char *status, bool ellipse, bool ephemeral, const char
>                  }
>          }
>  
> -        if (prev_ephemeral)
> -                IOVEC_SET_STRING(iovec[n++], "\r" ANSI_ERASE_TO_END_OF_LINE);
> +        if (prev_ephemeral) {
> +                if (is_ansi_console)
> +                        IOVEC_SET_STRING(iovec[n++], "\r" ANSI_ERASE_TO_END_OF_LINE);
> +                else
> +                        IOVEC_SET_STRING(iovec[n++], "\r");
> +        }
>          prev_ephemeral = ephemeral;
>  
>          if (status) {
> @@ -3220,8 +3260,22 @@ void columns_lines_cache_reset(int signum) {
>  bool on_tty(void) {
>          static int cached_on_tty = -1;
>  
> -        if (_unlikely_(cached_on_tty < 0))
> +        if (_unlikely_(cached_on_tty < 0)) {
>                  cached_on_tty = isatty(STDOUT_FILENO) > 0;
> +#if defined (__s390__) || defined (__s390x__)
> +                if (cached_on_tty) {
> +                        const char *e = getenv("TERM");
> +                        if (!e)
> +                                return cached_on_tty;
> +                        if (streq(e, "dumb") || strneq(e, "ibm3", 4)) {
> +                                char *mode = NULL;
> +                                int r = parse_env_file("/proc/cmdline", WHITESPACE, "conmode", &mode, NULL);
> +                                if (r < 0 || !mode || !streq(mode, "3270"))
> +                                        cached_on_tty = 0;
> +                        }
> +                }
> +#endif
> +        }
>  
>          return cached_on_tty;
>  }
> @@ -3711,7 +3765,25 @@ bool tty_is_vc_resolve(const char *tty) {
>  const char *default_term_for_tty(const char *tty) {
>          assert(tty);
>  
> -        return tty_is_vc_resolve(tty) ? "TERM=linux" : "TERM=vt102";
> +        if (tty_is_vc_resolve(tty))
> +                return "TERM=linux";
> +
> +        if (startswith(tty, "/dev/"))
> +                tty += 5;
> +
> +#if defined (__s390__) || defined (__s390x__)
> +        if (streq(tty, "ttyS0")) {
> +                char *mode = NULL;
> +                int r = parse_env_file("/proc/cmdline", WHITESPACE, "conmode", &mode, NULL);
> +                if (r < 0 || !mode || !streq(mode, "3270"))
> +                        return "TERM=dumb";
> +                if (streq(mode, "3270"))
> +                        return "TERM=ibm327x";
> +        }
> +        if (streq(tty, "ttyS1"))
> +                return "TERM=vt220";
> +#endif
> +        return "TERM=vt102";
>  }
>  
>  bool dirent_is_file(const struct dirent *de) {
> diff --git src/shared/util.h src/shared/util.h
> index 1796014..2dc918d 100644
> --- src/shared/util.h
> +++ src/shared/util.h
> @@ -446,6 +446,7 @@ unsigned lines(void);
>  void columns_lines_cache_reset(int _unused_ signum);
>  
>  bool on_tty(void);
> +bool ansi_console(int fd);
>  
>  static inline const char *ansi_highlight(void) {
>          return on_tty() ? ANSI_HIGHLIGHT_ON : "";
> -- 
> 1.7.9.2
> 
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> 


More information about the systemd-devel mailing list