Link against ncursesw

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

Link against ncursesw

Stanislav Ochotnicky-2
Fedora recently received a valid bugreport [1] showing problems with
pinentry-curses and  UTF-8 descriptions (most visible when someone has
UTF-8 in their gnupg names). Most simple reproducer is:

$ pinentry-curses
OK Your orders please
option lc-ctype=pl_PL.UTF-8
OK
setdesc ąćęłńóśźżĄĆĘŁŃÓŚŹŻ
OK
getpin
[pinentry shows garbled text in curses]

I prepared a patch that fixes this (attached), but I am not entirely
convinced it's the proper fix. That's mainly because I just replaced
ncurses linking with ncursesw. This is fine for most desktop systems
that have wide-char ncurses, but I assume there might be systems you
want to support without ncursesw.

Proper way to deal with this would probably be to fallback like this:
ncursesw -> ncurses -> curses

Apart from this does adding setlocale(LC_CTYPE, "") call seem
sensible?


[1] https://bugzilla.redhat.com/show_bug.cgi?id=704495

--
Stanislav Ochotnicky <[hidden email]>
Software Engineer - Base Operating Systems Brno

PGP: 7B087241
Red Hat Inc.                               http://cz.redhat.com

_______________________________________________
Gpa-dev mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gpa-dev

0001-Link-against-ncursesw-and-fix-display-of-wide-charac.patch (2K) Download Attachment
signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Link against ncursesw

Daiki Ueno
Hi,

Stanislav Ochotnicky <[hidden email]> writes:

> Fedora recently received a valid bugreport [1] showing problems with
> pinentry-curses and  UTF-8 descriptions (most visible when someone has
> UTF-8 in their gnupg names). Most simple reproducer is:
>
> $ pinentry-curses
> OK Your orders please
> option lc-ctype=pl_PL.UTF-8
> OK
> setdesc ąćęłńóśźżĄĆĘŁŃÓŚŹŻ
> OK
> getpin
> [pinentry shows garbled text in curses]
>
> I prepared a patch that fixes this (attached), but I am not entirely
> convinced it's the proper fix. That's mainly because I just replaced
> ncurses linking with ncursesw. This is fine for most desktop systems
> that have wide-char ncurses, but I assume there might be systems you
> want to support without ncursesw.
That reminds me of that I wrote a similar patch back in 2009:

http://article.gmane.org/gmane.comp.encryption.gpg.devel/15428

which also addressed an issue that text frames are not rendered
correctly when there is a double-width character on a line.

I'm attaching a revised (and minimized) patch.

> Apart from this does adding setlocale(LC_CTYPE, "") call seem
> sensible?

It would be good to respect lc-ctype option.


>From a8c704818d9e18e3be0388e0ceac4419c72c4b50 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <[hidden email]>
Date: Wed, 10 Aug 2011 12:50:43 +0900
Subject: [PATCH] Add wide-char support to pinentry-curses.

---
 configure.ac               |    2 +-
 m4/curses.m4               |   20 +++++-
 pinentry/pinentry-curses.c |  150 ++++++++++++++++++++++++++++++++++++-------
 3 files changed, 143 insertions(+), 29 deletions(-)

diff --git a/configure.ac b/configure.ac
index 4b8ed79..bcbe26e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -158,7 +158,7 @@ fi
 
 # Checks for header files.
 AC_HEADER_STDC
-AC_CHECK_HEADERS(string.h unistd.h langinfo.h termio.h locale.h utime.h)
+AC_CHECK_HEADERS(string.h unistd.h langinfo.h termio.h locale.h utime.h wchar.h)
 
 dnl Checks for library functions.
 AC_CHECK_FUNCS(seteuid stpcpy mmap)
diff --git a/m4/curses.m4 b/m4/curses.m4
index 1e1cb4f..3a01881 100644
--- a/m4/curses.m4
+++ b/m4/curses.m4
@@ -28,7 +28,13 @@ AC_DEFUN([IU_LIB_NCURSES], [
   AC_ARG_ENABLE(ncurses,    [  --disable-ncurses       don't prefer -lncurses over -lcurses],
               , enable_ncurses=yes)
   if test "$enable_ncurses" = yes; then
-    AC_CHECK_LIB(ncurses, initscr, LIBNCURSES="-lncurses")
+    AC_CHECK_LIB(ncursesw, initscr, LIBNCURSES="-lncursesw",
+      AC_CHECK_LIB(ncurses, initscr, LIBNCURSES="-lncurses"))
+    if test "$ac_cv_lib_ncursesw_initscr" = yes; then
+      have_ncursesw=yes
+    else
+      have_ncursesw=no
+    fi
     if test "$LIBNCURSES"; then
       # Use ncurses header files instead of the ordinary ones, if possible;
       # is there a better way of doing this, that avoids looking in specific
@@ -53,9 +59,14 @@ AC_DEFUN([IU_LIB_NCURSES], [
       else
  AC_CACHE_CHECK(for ncurses include dir,
        inetutils_cv_includedir_ncurses,
+          if test "$have_ncursesw" = yes; then
+            ncursesdir=ncursesw
+          else
+            ncursesdir=ncurses
+          fi
   for D in $includedir $prefix/include /local/include /usr/local/include /include /usr/include; do
-    if test -d $D/ncurses; then
-      inetutils_cv_includedir_ncurses="$D/ncurses"
+    if test -d $D/$ncursesdir; then
+      inetutils_cv_includedir_ncurses="$D/$ncursesdir"
       break
     fi
     test "$inetutils_cv_includedir_ncurses" \
@@ -68,6 +79,9 @@ AC_DEFUN([IU_LIB_NCURSES], [
         NCURSES_INCLUDE="-I$inetutils_cv_includedir_ncurses"
       fi
     fi
+    if test $have_ncursesw = yes; then
+      AC_DEFINE(HAVE_NCURSESW, 1, [Define if you have working ncursesw])
+    fi
   fi
   AC_SUBST(NCURSES_INCLUDE)
   AC_SUBST(LIBNCURSES)])dnl
diff --git a/pinentry/pinentry-curses.c b/pinentry/pinentry-curses.c
index 76ddbdd..585059f 100644
--- a/pinentry/pinentry-curses.c
+++ b/pinentry/pinentry-curses.c
@@ -42,6 +42,10 @@
 
 #include <memory.h>
 
+#ifdef HAVE_WCHAR_H
+#include <wchar.h>
+#endif /*HAVE_WCHAR_H*/
+
 #include "pinentry.h"
 
 /* FIXME: We should allow configuration of these button labels and in
@@ -94,6 +98,24 @@ struct dialog
 typedef struct dialog *dialog_t;
 
 
+#ifdef HAVE_NCURSESW
+typedef wchar_t CH;
+#define STRLEN(x) wcslen (x)
+#define ADDCH(x) addnwstr (&x, 1);
+#define CHWIDTH(x) wcwidth (x)
+#define NULLCH L'\0'
+#define NLCH L'\n'
+#define SPCH L' '
+#else
+typedef char CH;
+#define STRLEN(x) strlen (x)
+#define ADDCH(x) addch ((unsigned char) x)
+#define CHWIDTH(x) 1
+#define NULLCH '\0'
+#define NLCH '\n'
+#define SPCH ' '
+#endif
+
 /* Return the next line up to MAXLEN columns wide in START and LEN.
    The first invocation should have 0 as *LEN.  If the line ends with
    a \n, it is a normal line that will be continued.  If it is a '\0'
@@ -101,40 +123,95 @@ typedef struct dialog *dialog_t;
    there is a forced line break.  A full line is returned and will be
    continued in the next line.  */
 static void
-collect_line (int maxlen, char **start_p, int *len_p)
+collect_line (int maxwidth, wchar_t **start_p, int *len_p)
 {
   int last_space = 0;
   int len = *len_p;
-  char *end;
+  int width = 0;
+  CH *end;
 
   /* Skip to next line.  */
   *start_p += len;
   /* Skip leading space.  */
-  while (**start_p == ' ')
+  while (**start_p == SPCH)
     (*start_p)++;
 
   end = *start_p;
   len = 0;
 
-  while (len < maxlen - 1 && *end && *end != '\n')
+  while (width < maxwidth - 1 && *end != NULLCH && *end != NLCH)
     {
       len++;
       end++;
-      if (*end == ' ')
+      if (*end == SPCH)
  last_space = len;
+      width += CHWIDTH (*end);
     }
 
-  if (*end && *end != '\n' && last_space != 0)
+  if (*end != NULLCH && *end != NLCH && last_space != 0)
     {
       /* We reached the end of the available space, but still have
  characters to go in this line.  We can break the line into
  two parts at a space.  */
       len = last_space;
-      (*start_p)[len] = '\n';
+      (*start_p)[len] = NLCH;
     }
   *len_p = len + 1;
 }
 
+#ifdef HAVE_NCURSESW
+static CH *
+utf8_to_local (char *lc_ctype, char *string)
+{
+  mbstate_t ps;
+  size_t len;
+  char *local;
+  const char *p;
+  wchar_t *wcs = NULL;
+  char *old_ctype = NULL;
+
+  local = pinentry_utf8_to_local (lc_ctype, string);
+  if (!local)
+    return NULL;
+
+  old_ctype = strdup (setlocale (LC_CTYPE, NULL));
+  setlocale (LC_CTYPE, lc_ctype? lc_ctype : "");
+
+  p = local;
+  memset (&ps, 0, sizeof(mbstate_t));
+  len = mbsrtowcs (NULL, &p, strlen (string), &ps);
+  if (len == (size_t)-1)
+    {
+      free (local);
+      goto leave;
+    }
+  wcs = calloc (len + 1, sizeof(wchar_t));
+  if (!wcs)
+    {
+      free (local);
+      goto leave;
+    }
+
+  p = local;
+  memset (&ps, 0, sizeof(mbstate_t));
+  mbsrtowcs (wcs, &p, len, &ps);
+
+ leave:
+  if (old_ctype)
+    {
+      setlocale (LC_CTYPE, old_ctype);
+      free (old_ctype);
+    }
+
+  return wcs;
+}
+#else
+static CH *
+utf8_to_local (const char *lc_ctype, const char *string)
+{
+  return pinentry_utf8_to_local (lc_ctype, string);
+}
+#endif
 
 static int
 dialog_create (pinentry_t pinentry, dialog_t dialog)
@@ -148,16 +225,15 @@ dialog_create (pinentry_t pinentry, dialog_t dialog)
   int xpos;
   int description_x = 0;
   int error_x = 0;
-  char *description = NULL;
-  char *error = NULL;
-  char *prompt = NULL;
+  CH *description = NULL;
+  CH *error = NULL;
+  CH *prompt = NULL;
 
 #define COPY_OUT(what) \
   do \
     if (pinentry->what) \
       { \
-        what = pinentry_utf8_to_local (pinentry->lc_ctype, \
-       pinentry->what); \
+        what = utf8_to_local (pinentry->lc_ctype, pinentry->what); \
         if (!what) \
   { \
     err = 1; \
@@ -214,7 +290,7 @@ dialog_create (pinentry_t pinentry, dialog_t dialog)
   y = 1; /* Top frame.  */
   if (description)
     {
-      char *start = description;
+      CH *start = description;
       int len = 0;
 
       do
@@ -232,7 +308,7 @@ dialog_create (pinentry_t pinentry, dialog_t dialog)
     {
       if (error)
  {
-  char *p = error;
+  CH *p = error;
   int err_x = 0;
 
   while (*p)
@@ -287,7 +363,9 @@ dialog_create (pinentry_t pinentry, dialog_t dialog)
 
       new_x = MIN_PINENTRY_LENGTH;
       if (prompt)
- new_x += strlen (prompt) + 1; /* One space after prompt.  */
+ {
+  new_x += STRLEN (prompt) + 1; /* One space after prompt.  */
+ }
       if (new_x > size_x - 4)
  new_x = size_x - 4;
       if (new_x > x)
@@ -335,7 +413,7 @@ dialog_create (pinentry_t pinentry, dialog_t dialog)
   ypos++;
   if (description)
     {
-      char *start = description;
+      CH *start = description;
       int len = 0;
 
       do
@@ -347,9 +425,11 @@ dialog_create (pinentry_t pinentry, dialog_t dialog)
   addch (' ');
   collect_line (size_x - 4, &start, &len);
   for (i = 0; i < len - 1; i++)
-    addch ((unsigned char) start[i]);
-  if (start[len - 1] && start[len - 1] != '\n')
-    addch ((unsigned char) start[len - 1]);
+    {
+      ADDCH (start[i]);
+    }
+  if (start[len - 1] != NULLCH && start[len - 1] != NLCH)
+    ADDCH (start[len - 1]);
   ypos++;
  }
       while (start[len - 1]);
@@ -363,7 +443,7 @@ dialog_create (pinentry_t pinentry, dialog_t dialog)
 
       if (error)
  {
-  char *p = error;
+  CH *p = error;
   i = 0;
 
   while (*p)
@@ -378,11 +458,11 @@ dialog_create (pinentry_t pinentry, dialog_t dialog)
  }
       else
  standout ();
-      for (;*p && *p != '\n'; p++)
+      for (;*p && *p != NLCH; p++)
  if (i < x - 4)
   {
     i++;
-    addch ((unsigned char) *p);
+    ADDCH (*p);
   }
       if (USE_COLORS && pinentry->color_so != PINENTRY_COLOR_NONE)
  {
@@ -410,14 +490,16 @@ dialog_create (pinentry_t pinentry, dialog_t dialog)
       dialog->pin_size = x - 4;
       if (prompt)
  {
-  char *p = prompt;
-  i = strlen (prompt);
+  CH *p = prompt;
+  i = STRLEN (prompt);
   if (i > x - 4 - MIN_PINENTRY_LENGTH)
     i = x - 4 - MIN_PINENTRY_LENGTH;
   dialog->pin_x += i + 1;
   dialog->pin_size -= i + 1;
   while (i-- > 0)
-    addch ((unsigned char) *(p++));
+    {
+      ADDCH (*(p++));
+    }
   addch (' ');
  }
       for (i = 0; i < dialog->pin_size; i++)
@@ -631,6 +713,17 @@ dialog_run (pinentry_t pinentry, const char *tty_name, const char *tty_type)
   SCREEN *screen = 0;
   int done = 0;
   char *pin_utf8;
+#ifdef HAVE_NCURSESW
+  char *old_ctype = NULL;
+
+  if (pinentry->lc_ctype)
+    {
+      old_ctype = strdup (setlocale (LC_CTYPE, NULL));
+      setlocale (LC_CTYPE, pinentry->lc_ctype);
+    }
+  else
+    setlocale (LC_CTYPE, "");
+#endif
 
   /* Open the desired terminal if necessary.  */
   if (tty_name)
@@ -804,6 +897,13 @@ dialog_run (pinentry_t pinentry, const char *tty_name, const char *tty_type)
   if (screen)
     delscreen (screen);
 
+#ifdef HAVE_NCURSESW
+  if (old_ctype)
+    {
+      setlocale (LC_CTYPE, old_ctype);
+      free (old_ctype);
+    }
+#endif
   if (ttyfi)
     fclose (ttyfi);
   if (ttyfo)
--
1.7.6



Regards,
--
Daiki Ueno

_______________________________________________
Gpa-dev mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gpa-dev
Reply | Threaded
Open this post in threaded view
|

Re: Link against ncursesw

Werner Koch
On Wed, 10 Aug 2011 05:59, [hidden email] said:

> I'm attaching a revised (and minimized) patch.

Applied.


Salam-Shalom,

   Werner

--
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.


_______________________________________________
Gpa-dev mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gpa-dev
Reply | Threaded
Open this post in threaded view
|

Re: Link against ncursesw

Werner Koch
In reply to this post by Stanislav Ochotnicky-2
On Fri, 22 Jul 2011 14:30, [hidden email] said:

> I prepared a patch that fixes this (attached), but I am not entirely
> convinced it's the proper fix. That's mainly because I just replaced
> ncurses linking with ncursesw. This is fine for most desktop systems
> that have wide-char ncurses, but I assume there might be systems you

Do we still need this pacth after Ueno's patch?


BTW, please send pinentry related stuff in the future to gnupg-devel.
gpa--dev is only for GPA which is just one application using GnuPG.


Shalom-Salam,

   Werner


--
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.


_______________________________________________
Gpa-dev mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gpa-dev