Merge branch 'refs/heads/t/gui-improvements'
authorAndre Noll <maan@tuebingen.mpg.de>
Sun, 15 May 2016 13:30:20 +0000 (15:30 +0200)
committerAndre Noll <maan@tuebingen.mpg.de>
Sun, 15 May 2016 13:32:33 +0000 (15:32 +0200)
Improved wide-character support and fixes related to signal
handling. This topic branch was cooking in next for two months.

* refs/heads/t/gui-improvements:
  gui.c: Constify argument to find_cmd_byname().
  gui.c: Improve description of signal task.
  gui.c: Remove pointless return statement.
  gui.c: Remove silly warning on SIGINT.
  gui: Kill process group *before* shutting down curses.
  gui.c: Reset terminal on shutdown in external mode.
  gui: Avoid bad terminal state with xterm.
  gui.c: Constify local variables of add_spaces().
  Introduce sanitize_str().
  string.c: Simplify and rename wide character helpers.

NEWS.md
gui.c
string.c
string.h

diff --git a/NEWS.md b/NEWS.md
index 6ea6ecf..a0bec53 100644 (file)
--- a/NEWS.md
+++ b/NEWS.md
@@ -20,6 +20,7 @@ not mentioned here have accumulated and are also part of the release.
 - New mood methods: image_id and lyrics_id.
 - The manual and this NEWS file have been converted to markdown.
 - Cleanup of the wma decoder and bitstream code.
+- Improved wide-character support and fixes related to signal handling.
 
 Download: [tarball](./releases/paraslash-git.tar.bz2)
 
diff --git a/gui.c b/gui.c
index 9d96e70..b0eae64 100644 (file)
--- a/gui.c
+++ b/gui.c
@@ -146,7 +146,7 @@ struct exec_task {
        unsigned flags[2]; /* passed to for_each_line() */
 };
 
-static int find_cmd_byname(char *name)
+static int find_cmd_byname(const char *name)
 {
        int i;
 
@@ -246,8 +246,8 @@ static char *km_keyname(int c)
 /* Print given number of spaces to curses window. */
 static void add_spaces(WINDOW* win, unsigned int num)
 {
-       char space[] = "                                ";
-       unsigned sz = sizeof(space) - 1; /* number of spaces */
+       const char space[] = "                                ";
+       const unsigned sz = sizeof(space) - 1; /* number of spaces */
 
        while (num >= sz)  {
                waddstr(win, space);
@@ -255,8 +255,7 @@ static void add_spaces(WINDOW* win, unsigned int num)
        }
        if (num > 0) {
                assert(num < sz);
-               space[num] = '\0';
-               waddstr(win, space);
+               waddstr(win, space + sz - num);
        }
 }
 
@@ -264,41 +263,35 @@ static void add_spaces(WINDOW* win, unsigned int num)
  * print aligned string to curses window. This function always prints
  * exactly len chars.
  */
-static int align_str(WINDOW* win, char *str, unsigned int len,
+static int align_str(WINDOW* win, const char *str, unsigned int len,
                unsigned int align)
 {
-       int ret, i, num; /* of spaces */
+       int ret, num; /* of spaces */
        size_t width;
+       char *sstr; /* sanitized string */
 
        if (!win || !str)
                return 0;
-       ret = strwidth(str, &width);
+       ret = sanitize_str(str, len, &sstr, &width);
        if (ret < 0) {
                PARA_ERROR_LOG("%s\n", para_strerror(-ret));
                width = 0;
-               str[0] = '\0';
+               sstr = para_strdup(NULL);
        }
+       assert(width <= len);
        num = len - width;
-       if (num < 0) {
-               str[len] = '\0';
-               num = 0;
-       }
-       /* replace control characters by spaces */
-       for (i = 0; i < len && str[i]; i++) {
-               if (str[i] == '\n' || str[i] == '\r' || str[i] == '\f')
-                       str[i] = ' ';
-       }
        if (align == LEFT) {
-               waddstr(win, str);
+               waddstr(win, sstr);
                add_spaces(win, num);
        } else if (align == RIGHT) {
                add_spaces(win, num);
-               waddstr(win, str);
+               waddstr(win, sstr);
        } else {
                add_spaces(win, num / 2);
-               waddstr(win, str);
+               waddstr(win, sstr);
                add_spaces(win, num - num / 2);
        }
+       free(sstr);
        return 1;
 }
 
@@ -505,9 +498,18 @@ static __printf_2_3 void curses_log(int ll, const char *fmt,...)
 /** The log function of para_gui, always set to curses_log(). */
 __printf_2_3 void (*para_log)(int, const char*, ...) = curses_log;
 
+/* Call endwin() to reset the terminal into non-visual mode. */
 static void shutdown_curses(void)
 {
-       def_prog_mode();
+       /*
+        * If para_gui received a terminating signal in external mode, the
+        * terminal can be in an unusable state at this point because the child
+        * process might not have caught the signal. In this case endwin() has
+        * already been called and must not be called again. So we first return
+        * to program mode, then call endwin().
+        */
+       if (!curses_active())
+               reset_prog_mode();
        endwin();
 }
 
@@ -516,13 +518,22 @@ __noreturn __printf_2_3 static void die(int exit_code, const char* fmt, ...)
 {
        va_list argp;
 
+       /* Kill every process in our process group. */
+       para_sigaction(SIGTERM, SIG_IGN);
+       kill(0, SIGTERM);
+       /* Wait up to two seconds for child processes to die. */
+       alarm(2);
+       while (waitpid(0, NULL, 0) >= 0)
+               ; /* nothing */
+       alarm(0);
+       /* mousemask() exists only in ncurses */
+#ifdef NCURSES_MOUSE_VERSION
+       mousemask(~(mmask_t)0, NULL); /* Avoid bad terminal state with xterm. */
+#endif
        shutdown_curses();
        va_start(argp, fmt);
        vfprintf(stderr, fmt, argp);
        va_end(argp);
-       /* kill every process in the process group and exit */
-       para_sigaction(SIGTERM, SIG_IGN);
-       kill(0, SIGTERM);
        exit(exit_code);
 }
 
@@ -924,12 +935,7 @@ static int signal_post_select(struct sched *s, __a_unused void *context)
        switch (ret) {
        case SIGTERM:
                die(EXIT_FAILURE, "only the good die young (caught SIGTERM)\n");
-               return 1;
        case SIGINT:
-               PARA_WARNING_LOG("caught SIGINT, reset\n");
-               /* Nothing to do. SIGINT killed our child which gets noticed
-                * by do_select and resets everything.
-                */
                return 1;
        case SIGUSR1:
                PARA_NOTICE_LOG("got SIGUSR1, rereading configuration\n");
@@ -1485,9 +1491,9 @@ static int setup_tasks_and_schedule(void)
  * The exec task is responsible for printing the output of the currently
  * running executable to the bottom window.
  *
- * The signal task performs suitable actions according to any signals received.
- * For example it refreshes all windows on terminal size changes and resets the
- * terminal on \p SIGTERM.
+ * The signal task performs various actions according to signals received. For
+ * example, it reloads the configuration file on SIGUSR1, and it shuts down the
+ * curses system on SIGTERM to restore the terminal settings before exit.
  *
  * The input task reads single key strokes from stdin. For each key pressed, it
  * executes the command handler associated with this key.
index 701448e..e731bb4 100644 (file)
--- a/string.c
+++ b/string.c
@@ -955,36 +955,24 @@ static bool utf8_mode(void)
        return have_utf8;
 }
 
-/*
- * glibc's wcswidth returns -1 if the string contains a tab character, which
- * makes the function next to useless. The two functions below are taken from
- * mutt.
- */
-
-#define IsWPrint(wc) (iswprint(wc) || wc >= 0xa0)
-
-static int mutt_wcwidth(wchar_t wc, size_t pos)
+static int xwcwidth(wchar_t wc, size_t pos)
 {
        int n;
 
+       /* special-case for tab */
        if (wc == 0x09) /* tab */
                return (pos | 7) + 1 - pos;
        n = wcwidth(wc);
-       if (IsWPrint(wc) && n > 0)
-               return n;
-       if (!(wc & ~0x7f))
-               return 2;
-       if (!(wc & ~0xffff))
-               return 6;
-       return 10;
+       /* wcswidth() returns -1 for non-printable characters */
+       return n >= 0? n : 1;
 }
 
-static size_t mutt_wcswidth(const wchar_t *s, size_t n)
+static size_t xwcswidth(const wchar_t *s, size_t n)
 {
        size_t w = 0;
 
        while (n--)
-               w += mutt_wcwidth(*s++, w);
+               w += xwcwidth(*s++, w);
        return w;
 }
 
@@ -1029,7 +1017,7 @@ int skip_cells(const char *s, size_t cells_to_skip, size_t *bytes_to_skip)
                if (mbret == (size_t)-1 || mbret == (size_t)-2)
                        return -ERRNO_TO_PARA_ERROR(EILSEQ);
                bytes_parsed += mbret;
-               cells_skipped += mutt_wcwidth(wc, cells_skipped);
+               cells_skipped += xwcwidth(wc, cells_skipped);
        }
        *bytes_to_skip = bytes_parsed;
        return 1;
@@ -1078,7 +1066,70 @@ __must_check int strwidth(const char *s, size_t *result)
        memset(&state, 0, sizeof(state));
        num_wchars = mbsrtowcs(dest, &src, num_wchars, &state);
        assert(num_wchars > 0 && num_wchars != (size_t)-1);
-       *result = mutt_wcswidth(dest, num_wchars);
+       *result = xwcswidth(dest, num_wchars);
        free(dest);
        return 1;
 }
+
+/**
+ * Truncate and sanitize a (wide character) string.
+ *
+ * This replaces all non-printable characters by spaces and makes sure that the
+ * modified string does not exceed the given maximal width.
+ *
+ * \param src The source string in multi-byte form.
+ * \param max_width The maximal number of cells the result may occupy.
+ * \param result Sanitized multi-byte string, must be freed by caller.
+ * \param width The width of the sanitized string, always <= max_width.
+ *
+ * The function is wide-character aware but falls back to C strings for
+ * non-UTF-8 locales.
+ *
+ * \return Standard. On success, *result points to a sanitized copy of the
+ * given string. This copy was allocated with malloc() and should hence be
+ * freed when the caller is no longer interested in the result.
+ *
+ * The function fails if the given string contains an invalid multibyte
+ * sequence. In this case, *result is set to NULL, and *width to zero.
+ */
+__must_check int sanitize_str(const char *src, size_t max_width,
+               char **result, size_t *width)
+{
+       mbstate_t state;
+       static wchar_t *wcs;
+       size_t num_wchars, n;
+
+       if (!utf8_mode()) {
+               *result = para_strdup(src);
+               /* replace non-printable characters by spaces */
+               for (n = 0; n < max_width && src[n]; n++) {
+                       if (!isprint((unsigned char)src[n]))
+                               (*result)[n] = ' ';
+               }
+               (*result)[n] = '\0';
+               *width = n;
+               return 0;
+       }
+       *result = NULL;
+       *width = 0;
+       memset(&state, 0, sizeof(state));
+       num_wchars = mbsrtowcs(NULL, &src, 0, &state);
+       if (num_wchars == (size_t)-1)
+               return -ERRNO_TO_PARA_ERROR(errno);
+       wcs = para_malloc((num_wchars + 1) * sizeof(*wcs));
+       memset(&state, 0, sizeof(state));
+       num_wchars = mbsrtowcs(wcs, &src, num_wchars + 1, &state);
+       assert(num_wchars != (size_t)-1);
+       for (n = 0; n < num_wchars && *width < max_width; n++) {
+               if (!iswprint(wcs[n]))
+                       wcs[n] = L' ';
+               *width += xwcwidth(wcs[n], *width);
+       }
+       wcs[n] = L'\0';
+       n = wcstombs(NULL, wcs, 0) + 1;
+       *result = para_malloc(n);
+       num_wchars = wcstombs(*result, wcs, n);
+       assert(num_wchars != (size_t)-1);
+       free(wcs);
+       return 1;
+}
index 61bb7c2..aa8292f 100644 (file)
--- a/string.h
+++ b/string.h
@@ -101,3 +101,5 @@ char *safe_strdup(const char *src, size_t len);
 char *key_value_copy(const char *src, size_t len, const char *key);
 int skip_cells(const char *s, size_t cells_to_skip, size_t *result);
 __must_check int strwidth(const char *s, size_t *result);
+__must_check int sanitize_str(const char *src, size_t max_width,
+               char **result, size_t *width);