[RFA] Handle older iconv programs

[RFA] Handle older iconv programs

[Doug Evans]

Hi.

Older iconv programs emit the human readable form even if stdout is not a tty.

Note: This patch requires a libiberty fix:
http://gcc.gnu.org/ml/gcc-patches/2009-12/msg00380.html

Ok to check in?
[after the libiberty patch goes in]

2009-12-07  Doug Evans  <dje@google.com>

	* charset.c: Include environ.h.
	(ignore_line_p): New function.
	(find_charset_names): Handle older versions of iconv that print
	human-readable output even if stdout is not a tty.

==== //depot2/gcctools/google_vendor_src_branch/gdb/gdb-7.0.x-prod/gdb/charset.c#1 - /usr/local/google/home/dje/ctools/gdb/7.0.x-prod/depot2/gcctools/google_vendor_src_branch/gdb/gdb-7.0.x-prod/gdb/charset.c ====
--- /tmp/g4-67641/cache/depot2/gcctools/google_vendor_src_branch/gdb/gdb-7.0.x-prod/gdb/charset.c#1	2009-12-06 12:30:44.000000000 -0800
+++ /usr/local/google/home/dje/ctools/gdb/7.0.x-prod/depot2/gcctools/google_vendor_src_branch/gdb/gdb-7.0.x-prod/gdb/charset.c	2009-12-06 12:01:12.000000000 -0800
@@ -25,6 +25,7 @@
 #include "gdb_wait.h"
 #include "charset-list.h"
 #include "vec.h"
+#include "environ.h"
 
 #include <stddef.h>
 #include "gdb_string.h"
@@ -705,6 +706,35 @@
 
 #else
 
+/* Return non-zero if LINE (output from iconv) should be ignored.
+   Older iconv programs (e.g. 2.2.2) include the human readable
+   introduction even when stdout is not a tty.  Newer versions omit
+   the intro if stdout is not a tty.  */
+
+static int
+ignore_line_p (const char *line)
+{
+  /* This table is used to filter the output.  If this text appears
+     anywhere in the line, it is ignored (strstr is used).  */
+  static const char * const ignore_lines[] =
+    {
+      "The following",
+      "not necessarily",
+      "the FROM and TO",
+      "listed with several",
+      NULL
+    };
+  int i;
+
+  for (i = 0; ignore_lines[i] != NULL; ++i)
+    {
+      if (strstr (line, ignore_lines[i]) != NULL)
+	return 1;
+    }
+
+  return 0;
+}
+
 static void
 find_charset_names (void)
 {
@@ -712,6 +742,15 @@
   char *args[3];
   int err, status;
   int fail = 1;
+  struct gdb_environ *iconv_env;
+
+  /* Older iconvs, e.g. 2.2.2, don't omit the intro text if stdout is not
+     a tty.  We need to recognize it and ignore it.  This text is subject
+     to translation, so force LANGUAGE=en.  */
+  iconv_env = make_environ ();
+  init_environ (iconv_env);
+  set_in_environ (iconv_env, "LANGUAGE", "en");
+  set_in_environ (iconv_env, "LC_ALL", "C");
 
   child = pex_init (0, "iconv", NULL);
 
@@ -719,14 +758,16 @@
   args[1] = "-l";
   args[2] = NULL;
   /* Note that we simply ignore errors here.  */
-  if (!pex_run (child, PEX_SEARCH | PEX_STDERR_TO_STDOUT, "iconv",
-		args, NULL, NULL, &err))
+  if (!pex_run_in_environment (child, PEX_SEARCH | PEX_STDERR_TO_STDOUT,
+			       "iconv", args, environ_vector (iconv_env),
+			       NULL, NULL, &err))
     {
       FILE *in = pex_read_output (child, 0);
 
       /* POSIX says that iconv -l uses an unspecified format.  We
 	 parse the glibc and libiconv formats; feel free to add others
 	 as needed.  */
+
       while (!feof (in))
 	{
 	  /* The size of buf is chosen arbitrarily.  */
@@ -740,6 +781,9 @@
 	  len = strlen (r);
 	  if (len <= 3)
 	    continue;
+	  if (ignore_line_p (r))
+	    continue;
+
 	  /* Strip off the newline.  */
 	  --len;
 	  /* Strip off one or two '/'s.  glibc will print lines like
@@ -751,15 +795,21 @@
 	  buf[len] = '\0';
 
 	  /* libiconv will print multiple entries per line, separated
-	     by spaces.  */
+	     by spaces.  Older iconvs will print multiple entries per line,
+	     indented by two spaces, and separated by ", "
+	     (i.e. the human readable form).  */
 	  start = buf;
 	  while (1)
 	    {
 	      int keep_going;
 	      char *p;
 
-	      /* Find the next space, or end-of-line.  */
-	      for (p = start; *p && *p != ' '; ++p)
+	      /* Skip leading blanks.  */
+	      for (p = start; *p && *p == ' '; ++p)
+		;
+	      start = p;
+	      /* Find the next space, comma, or end-of-line.  */
+	      for ( ; *p && *p != ' ' && *p != ','; ++p)
 		;
 	      /* Ignore an empty result.  */
 	      if (p == start)
@@ -782,6 +832,7 @@
     }
 
   pex_free (child);
+  free_environ (iconv_env);
 
   if (fail)
     {

Re: [RFA] Handle older iconv programs

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> +  set_in_environ (iconv_env, "LANGUAGE", "en");

I think "C" would be preferable here.
Or, just remove LANGUAGE from the environment.

Otherwise this seems fine to me.

Tom

Re: [RFA] Handle older iconv programs

[Paul Pluzhnikov]

On Mon, Dec 7, 2009 at 12:36 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> +  set_in_environ (iconv_env, "LANGUAGE", "en");
>
> I think "C" would be preferable here.
> Or, just remove LANGUAGE from the environment.

I am seeing a crash which I think directly relates to this patch:

(top) r
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/grte/v1/lib64/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
*__GI_getenv (name=0x7ffff6cb8f85 "NGUAGE") at ../sysdeps/generic/getenv.c:93
93                if (name_start == ep_start && !strncmp (*ep + 2, name, len)
(top) bt
#0  *__GI_getenv (name=0x7ffff6cb8f85 "NGUAGE") at
../sysdeps/generic/getenv.c:93
#1  0x00007ffff6bcbf8f in guess_category_value (domainname=0x9cffb0 "gdb",
    msgid1=0x77d8a0 "The `host character set' is the one used by the
system GDB is running on.\nYou may only use supersets of ASCII for
your host character set; GDB does\nnot support any others.\nTo see a
list of the charact"..., msgid2=0x0, plural=0, n=0,
    category=8) at dcigettext.c:1107
#2  __dcigettext (domainname=0x9cffb0 "gdb",
    msgid1=0x77d8a0 "The `host character set' is the one used by the
system GDB is running on.\nYou may only use supersets of ASCII for
your host character set; GDB does\nnot support any others.\nTo see a
list of the charact"..., msgid2=0x0, plural=0, n=0,
    category=8) at dcigettext.c:533
#3  0x00000000004abd33 in _initialize_charset () at ../../src/gdb/charset.c:898
#4  0x000000000041d5d8 in initialize_all_files () at init.c:160
#5  0x000000000040c289 in gdb_init (argv0=0x7fffffffddf2
"/home/ppluzhnikov/bin/gdb64-cvs") at ../../src/gdb/top.c:1729
#6  0x0000000000400fa5 in captured_main (data=0x7fffffffd940) at
../../src/gdb/main.c:688
#7  0x00000000004fdbd9 in catch_errors (func=0x40068d <captured_main>,
func_args=0x7fffffffd940, errstring=0x74f01f "", mask=6) at
../../src/gdb/exceptions.c:510
#8  0x00000000004016d4 in gdb_main (args=0x7fffffffd940) at
../../src/gdb/main.c:911
#9  0x00000000004003a4 in main (argc=1, argv=0x7fffffffda38) at
../../src/gdb/gdb.c:33

Environment has been stepped on:

(top) p __environ[0]
$4 = 0x6e0a73656f642042 <Address 0x6e0a73656f642042 out of bounds>
(top) x/s __environ
0x9fa370:        "B does\nnot support any others.\nTo see a list of
the character sets GDB supports, type `set charset <TAB>'."

Still digging into the cause ...

-- 
Paul Pluzhnikov

Re: [RFA] Handle older iconv programs

[Doug Evans]

On Fri, Dec 11, 2009 at 1:03 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> On Mon, Dec 7, 2009 at 12:36 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>>
>> Doug> +  set_in_environ (iconv_env, "LANGUAGE", "en");
>>
>> I think "C" would be preferable here.
>> Or, just remove LANGUAGE from the environment.
>
> I am seeing a crash which I think directly relates to this patch:
>
> (top) r
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/usr/grte/v1/lib64/libthread_db.so.1".
>
> Program received signal SIGSEGV, Segmentation fault.
> *__GI_getenv (name=0x7ffff6cb8f85 "NGUAGE") at ../sysdeps/generic/getenv.c:93
> 93                if (name_start == ep_start && !strncmp (*ep + 2, name, len)

For reference sake, have you applied the accompanying patch to libiberty?

2009-12-07  Doug Evans  <dje@google.com>

        * pex-unix.c (pex_unix_exec_child): Save/restore environ.

Re: [RFA] Handle older iconv programs

[Paul Pluzhnikov]

On Fri, Dec 11, 2009 at 1:06 PM, Doug Evans <dje@google.com> wrote:

> For reference sake, have you applied the accompanying patch to libiberty?

Oops. Sorry for the noise.

Updating libiberty makes the problem go away :-)

Thanks,
-- 
Paul Pluzhnikov