Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [doc RFA]: New option --with-iconv-bin
@ 2011-05-09 18:20 Doug Evans
  2011-05-09 19:09 ` Eli Zaretskii
  2011-05-09 19:41 ` Tom Tromey
  0 siblings, 2 replies; 7+ messages in thread
From: Doug Evans @ 2011-05-09 18:20 UTC (permalink / raw)
  To: gdb-patches

Hi.

ref: http://sourceware.org/ml/gdb-patches/2011-05/msg00169.html

Based on the feedback I received, I will be checking this patch in.

Eli: doc RFA?  [thanks]

2011-05-09  Doug Evans  <dje@google.com>

	* NEWS: Mention --with-iconv-bin.
	* configure.ac: New option --with-iconv-bin.
	* configure: Regenerate.
	* config.in: Regenerate.
	* defs.h (relocate_gdb_directory): Declare.
	* main.c (relocate_gdb_directory): Renamed from relocate_directory
	and exported.  All callers updated.
	* charset.c (find_charset_names): Use --with-iconv-bin if specified.

	doc/
	gdb.texinfo (Requirements): Fix typo.  Mention --with-iconv-bin.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.437
diff -u -p -r1.437 NEWS
--- NEWS	6 May 2011 18:46:31 -0000	1.437
+++ NEWS	9 May 2011 18:08:10 -0000
@@ -3,6 +3,9 @@
 
 *** Changes since GDB 7.3
 
+* New configure option --with-iconv-bin to specify where to find the
+  iconv program.
+
 * When natively debugging programs on PowerPC BookE processors running
   a Linux kernel version 2.6.34 or later, GDB supports masked hardware
   watchpoints, which specify a mask in addition to an address to watch.
Index: charset.c
===================================================================
RCS file: /cvs/src/src/gdb/charset.c,v
retrieving revision 1.44
diff -u -p -r1.44 charset.c
--- charset.c	21 Apr 2011 14:26:38 -0000	1.44
+++ charset.c	9 May 2011 18:08:10 -0000
@@ -799,7 +799,9 @@ find_charset_names (void)
   char *args[3];
   int err, status;
   int fail = 1;
+  int flags;
   struct gdb_environ *iconv_env;
+  char *iconv_program;
 
   /* 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
@@ -811,12 +813,26 @@ find_charset_names (void)
 
   child = pex_init (PEX_USE_PIPES, "iconv", NULL);
 
-  args[0] = "iconv";
+#ifdef ICONV_BIN
+  {
+    char *iconv_dir = relocate_gdb_directory (ICONV_BIN,
+					      ICONV_BIN_RELOCATABLE);
+    iconv_program = concat (iconv_dir, SLASH_STRING, "iconv", NULL);
+    free (iconv_dir);
+  }
+#else
+  iconv_program = xstrdup ("iconv");
+#endif
+  args[0] = iconv_program;
   args[1] = "-l";
   args[2] = NULL;
+  flags = PEX_STDERR_TO_STDOUT;
+#ifndef ICONV_BIN
+  flags |= PEX_SEARCH;
+#endif
   /* Note that we simply ignore errors here.  */
-  if (!pex_run_in_environment (child, PEX_SEARCH | PEX_STDERR_TO_STDOUT,
-			       "iconv", args, environ_vector (iconv_env),
+  if (!pex_run_in_environment (child, flags,
+			       args[0], args, environ_vector (iconv_env),
 			       NULL, NULL, &err))
     {
       FILE *in = pex_read_output (child, 0);
@@ -888,6 +904,7 @@ find_charset_names (void)
 
     }
 
+  xfree (iconv_program);
   pex_free (child);
   free_environ (iconv_env);
 
Index: configure.ac
===================================================================
RCS file: /cvs/src/src/gdb/configure.ac,v
retrieving revision 1.144
diff -u -p -r1.144 configure.ac
--- configure.ac	17 Mar 2011 13:19:10 -0000	1.144
+++ configure.ac	9 May 2011 18:08:11 -0000
@@ -433,6 +433,29 @@ AC_SEARCH_LIBS(dlgetmodinfo, [dl xpdl])
 
 AM_ICONV
 
+# GDB may fork/exec the iconv program to get the list of supported character
+# sets.  Allow the user to specify where to find it.
+# There are several factors affecting the choice of option name:
+# - There is already --with-libiconv-prefix but we can't use it, it specifies
+#   the build-time location of libiconv files.
+# - The program we need to find is iconv, which comes with glibc.  The user
+#   doesn't necessarily have libiconv installed.  Therefore naming this
+#   --with-libiconv-foo feels wrong.
+# - We want the path to be relocatable, but GDB_AC_DEFINE_RELOCATABLE is
+#   defined to work on directories not files (though it really doesn't know
+#   the difference).
+# - Calling this --with-iconv-prefix is perceived to cause too much confusion
+#   with --with-libiconv-prefix.
+# Putting these together is why the option name is --with-iconv-bin.
+
+AC_ARG_WITH(iconv-bin,
+AS_HELP_STRING([--with-iconv-bin=PATH], [specify where to find the iconv program]),
+[iconv_bin="${withval}"
+ AC_DEFINE_UNQUOTED([ICONV_BIN], ["${iconv_bin}"],
+                    [Path of directory of iconv program.])
+ GDB_AC_DEFINE_RELOCATABLE(ICONV_BIN, iconv, ${iconv_bin})
+])
+
 # On alpha-osf, it appears that libtermcap and libcurses are not compatible.
 # There is a very specific comment in /usr/include/curses.h explaining that
 # termcap routines built into libcurses must not be used.
Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.291
diff -u -p -r1.291 defs.h
--- defs.h	25 Apr 2011 18:28:52 -0000	1.291
+++ defs.h	9 May 2011 18:08:11 -0000
@@ -282,6 +282,12 @@ struct breakpoint;
 struct frame_info;
 struct gdbarch;
 
+/* From main.c.  */
+
+/* This really belong in utils.c (path-utils.c?), but it references some
+   globals that are currently only available to main.c.  */
+extern char *relocate_gdb_directory (const char *initial, int flag);
+
 /* From utils.c */
 
 extern void initialize_utils (void);
Index: main.c
===================================================================
RCS file: /cvs/src/src/gdb/main.c,v
retrieving revision 1.93
diff -u -p -r1.93 main.c
--- main.c	7 Mar 2011 18:34:31 -0000	1.93
+++ main.c	9 May 2011 18:08:11 -0000
@@ -104,6 +104,7 @@ extern char *external_editor_command;
    file or directory.  FLAG is true if the value is relocatable, false
    otherwise.  Returns a newly allocated string; this may return NULL
    under the same conditions as make_relative_prefix.  */
+
 static char *
 relocate_path (const char *progname, const char *initial, int flag)
 {
@@ -117,12 +118,13 @@ relocate_path (const char *progname, con
    the result is a directory, it is used; otherwise, INITIAL is used.
    The chosen directory is then canonicalized using lrealpath.  This
    function always returns a newly-allocated string.  */
-static char *
-relocate_directory (const char *progname, const char *initial, int flag)
+
+char *
+relocate_gdb_directory (const char *initial, int flag)
 {
   char *dir;
 
-  dir = relocate_path (progname, initial, flag);
+  dir = relocate_path (gdb_program_name, initial, flag);
   if (dir)
     {
       struct stat s;
@@ -342,22 +344,21 @@ captured_main (void *data)
   current_directory = gdb_dirbuf;
 
   /* Set the sysroot path.  */
-  gdb_sysroot = relocate_directory (argv[0], TARGET_SYSTEM_ROOT,
-				    TARGET_SYSTEM_ROOT_RELOCATABLE);
+  gdb_sysroot = relocate_gdb_directory (TARGET_SYSTEM_ROOT,
+					TARGET_SYSTEM_ROOT_RELOCATABLE);
 
-  debug_file_directory = relocate_directory (argv[0], DEBUGDIR,
-					     DEBUGDIR_RELOCATABLE);
+  debug_file_directory = relocate_gdb_directory (DEBUGDIR,
+						 DEBUGDIR_RELOCATABLE);
 
-  gdb_datadir = relocate_directory (argv[0], GDB_DATADIR,
-				    GDB_DATADIR_RELOCATABLE);
+  gdb_datadir = relocate_gdb_directory (GDB_DATADIR,
+					GDB_DATADIR_RELOCATABLE);
 
 #ifdef WITH_PYTHON_PATH
   {
     /* For later use in helping Python find itself.  */
     char *tmp = concat (WITH_PYTHON_PATH, SLASH_STRING, "lib", NULL);
 
-    python_libdir = relocate_directory (argv[0], tmp,
-					PYTHON_PATH_RELOCATABLE);
+    python_libdir = relocate_gdb_directory (tmp, PYTHON_PATH_RELOCATABLE);
     xfree (tmp);
   }
 #endif
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.830
diff -u -p -r1.830 gdb.texinfo
--- doc/gdb.texinfo	6 May 2011 18:46:33 -0000	1.830
+++ doc/gdb.texinfo	9 May 2011 18:08:11 -0000
@@ -31214,7 +31214,12 @@ Sets}) require a functioning @code{iconv
 on a GNU system, then this is provided by the GNU C Library.  Some
 other systems also provide a working @code{iconv}.
 
-On systems with @code{iconv}, you can install GNU Libiconv.  If you
+If the GNU C Library you are using is installed in a non-standard place,
+you will need to tell @value{GDBN} where to find the @code{iconv} program.
+This is done with @option{--with-iconv-bin} which specifies the directory
+that contains the @code{iconv} program.
+
+On systems without @code{iconv}, you can install GNU Libiconv.  If you
 have previously installed Libiconv, you can use the
 @option{--with-libiconv-prefix} option to configure.
 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [doc RFA]: New option --with-iconv-bin
  2011-05-09 18:20 [doc RFA]: New option --with-iconv-bin Doug Evans
@ 2011-05-09 19:09 ` Eli Zaretskii
  2011-05-09 20:34   ` Doug Evans
       [not found]   ` <BANLkTikvtS3eSk_8e+40X5ky9h_HEX5VNQ@mail.gmail.com>
  2011-05-09 19:41 ` Tom Tromey
  1 sibling, 2 replies; 7+ messages in thread
From: Eli Zaretskii @ 2011-05-09 19:09 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

> Date: Mon,  9 May 2011 11:19:47 -0700 (PDT)
> From: dje@google.com (Doug Evans)
> 
>  *** Changes since GDB 7.3
>  
> +* New configure option --with-iconv-bin to specify where to find the
> +  iconv program.

Should we tell here what `iconv' is used for?  Users might not know
whether they should bother about this option.

> +If the GNU C Library you are using is installed in a non-standard place,
> +you will need to tell @value{GDBN} where to find the @code{iconv} program.
> +This is done with @option{--with-iconv-bin} which specifies the directory
> +that contains the @code{iconv} program.

I'm confused: what does `iconv's location have to do with the place
where glibc is installed?

Why not just say "if your @command{iconv} program is installed in a
non-standard place, you will need to tell @value{GDBN} where to find
it."?


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [doc RFA]: New option --with-iconv-bin
  2011-05-09 18:20 [doc RFA]: New option --with-iconv-bin Doug Evans
  2011-05-09 19:09 ` Eli Zaretskii
@ 2011-05-09 19:41 ` Tom Tromey
  1 sibling, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2011-05-09 19:41 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

Doug> +    free (iconv_dir);

xfree

Tom


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [doc RFA]: New option --with-iconv-bin
  2011-05-09 19:09 ` Eli Zaretskii
@ 2011-05-09 20:34   ` Doug Evans
       [not found]   ` <BANLkTikvtS3eSk_8e+40X5ky9h_HEX5VNQ@mail.gmail.com>
  1 sibling, 0 replies; 7+ messages in thread
From: Doug Evans @ 2011-05-09 20:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

[Sorry for the resend.]

On Mon, May 9, 2011 at 12:08 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Mon,  9 May 2011 11:19:47 -0700 (PDT)
>> From: dje@google.com (Doug Evans)
>>
>>  *** Changes since GDB 7.3
>>
>> +* New configure option --with-iconv-bin to specify where to find the
>> +  iconv program.
>
> Should we tell here what `iconv' is used for?  Users might not know
> whether they should bother about this option.

Sure.  Though there is the gdb.texinfo entry for elaboration.

>> +If the GNU C Library you are using is installed in a non-standard place,
>> +you will need to tell @value{GDBN} where to find the @code{iconv} program.
>> +This is done with @option{--with-iconv-bin} which specifies the directory
>> +that contains the @code{iconv} program.
>
> I'm confused: what does `iconv's location have to do with the place
> where glibc is installed?
>
> Why not just say "if your @command{iconv} program is installed in a
> non-standard place, you will need to tell @value{GDBN} where to find
> it."?

The iconv program comes from glibc, but you're right, it doesn't
*necessarily* come from there.

How about this?

free -> xfree fixed as well

[-- Attachment #2: gdb-110509-with-iconv-bin-2.patch.txt --]
[-- Type: text/plain, Size: 8556 bytes --]

2011-05-09  Doug Evans  <dje@google.com>

	* NEWS: Mention --with-iconv-bin.
	* configure.ac: New option --with-iconv-bin.
	* configure: Regenerate.
	* config.in: Regenerate.
	* defs.h (relocate_gdb_directory): Declare.
	* main.c (relocate_gdb_directory): Renamed from relocate_directory
	and exported.  All callers updated.
	* charset.c (find_charset_names): Use --with-iconv-bin if specified.

	doc/
	gdb.texinfo (Requirements): Fix typo.  Mention --with-iconv-bin.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.437
diff -u -p -d -u -r1.437 NEWS
--- NEWS	6 May 2011 18:46:31 -0000	1.437
+++ NEWS	9 May 2011 19:49:47 -0000
@@ -3,6 +3,12 @@
 
 *** Changes since GDB 7.3
 
+* New configure option --with-iconv-bin.
+  When using the internationalization support like the one in the GNU C
+  library, GDB will invoke the "iconv" program to get a list of supported
+  character sets.  If this program lives in a non-standard location, one can
+  use this option to specify where to find it.
+
 * When natively debugging programs on PowerPC BookE processors running
   a Linux kernel version 2.6.34 or later, GDB supports masked hardware
   watchpoints, which specify a mask in addition to an address to watch.
Index: charset.c
===================================================================
RCS file: /cvs/src/src/gdb/charset.c,v
retrieving revision 1.44
diff -u -p -d -u -r1.44 charset.c
--- charset.c	21 Apr 2011 14:26:38 -0000	1.44
+++ charset.c	9 May 2011 19:49:47 -0000
@@ -799,7 +799,9 @@ find_charset_names (void)
   char *args[3];
   int err, status;
   int fail = 1;
+  int flags;
   struct gdb_environ *iconv_env;
+  char *iconv_program;
 
   /* 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
@@ -811,12 +813,26 @@ find_charset_names (void)
 
   child = pex_init (PEX_USE_PIPES, "iconv", NULL);
 
-  args[0] = "iconv";
+#ifdef ICONV_BIN
+  {
+    char *iconv_dir = relocate_gdb_directory (ICONV_BIN,
+					      ICONV_BIN_RELOCATABLE);
+    iconv_program = concat (iconv_dir, SLASH_STRING, "iconv", NULL);
+    xfree (iconv_dir);
+  }
+#else
+  iconv_program = xstrdup ("iconv");
+#endif
+  args[0] = iconv_program;
   args[1] = "-l";
   args[2] = NULL;
+  flags = PEX_STDERR_TO_STDOUT;
+#ifndef ICONV_BIN
+  flags |= PEX_SEARCH;
+#endif
   /* Note that we simply ignore errors here.  */
-  if (!pex_run_in_environment (child, PEX_SEARCH | PEX_STDERR_TO_STDOUT,
-			       "iconv", args, environ_vector (iconv_env),
+  if (!pex_run_in_environment (child, flags,
+			       args[0], args, environ_vector (iconv_env),
 			       NULL, NULL, &err))
     {
       FILE *in = pex_read_output (child, 0);
@@ -888,6 +904,7 @@ find_charset_names (void)
 
     }
 
+  xfree (iconv_program);
   pex_free (child);
   free_environ (iconv_env);
 
Index: configure.ac
===================================================================
RCS file: /cvs/src/src/gdb/configure.ac,v
retrieving revision 1.144
diff -u -p -d -u -r1.144 configure.ac
--- configure.ac	17 Mar 2011 13:19:10 -0000	1.144
+++ configure.ac	9 May 2011 19:49:47 -0000
@@ -433,6 +433,29 @@ AC_SEARCH_LIBS(dlgetmodinfo, [dl xpdl])
 
 AM_ICONV
 
+# GDB may fork/exec the iconv program to get the list of supported character
+# sets.  Allow the user to specify where to find it.
+# There are several factors affecting the choice of option name:
+# - There is already --with-libiconv-prefix but we can't use it, it specifies
+#   the build-time location of libiconv files.
+# - The program we need to find is iconv, which comes with glibc.  The user
+#   doesn't necessarily have libiconv installed.  Therefore naming this
+#   --with-libiconv-foo feels wrong.
+# - We want the path to be relocatable, but GDB_AC_DEFINE_RELOCATABLE is
+#   defined to work on directories not files (though it really doesn't know
+#   the difference).
+# - Calling this --with-iconv-prefix is perceived to cause too much confusion
+#   with --with-libiconv-prefix.
+# Putting these together is why the option name is --with-iconv-bin.
+
+AC_ARG_WITH(iconv-bin,
+AS_HELP_STRING([--with-iconv-bin=PATH], [specify where to find the iconv program]),
+[iconv_bin="${withval}"
+ AC_DEFINE_UNQUOTED([ICONV_BIN], ["${iconv_bin}"],
+                    [Path of directory of iconv program.])
+ GDB_AC_DEFINE_RELOCATABLE(ICONV_BIN, iconv, ${iconv_bin})
+])
+
 # On alpha-osf, it appears that libtermcap and libcurses are not compatible.
 # There is a very specific comment in /usr/include/curses.h explaining that
 # termcap routines built into libcurses must not be used.
Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.291
diff -u -p -d -u -r1.291 defs.h
--- defs.h	25 Apr 2011 18:28:52 -0000	1.291
+++ defs.h	9 May 2011 19:49:47 -0000
@@ -282,6 +282,12 @@ struct breakpoint;
 struct frame_info;
 struct gdbarch;
 
+/* From main.c.  */
+
+/* This really belong in utils.c (path-utils.c?), but it references some
+   globals that are currently only available to main.c.  */
+extern char *relocate_gdb_directory (const char *initial, int flag);
+
 /* From utils.c */
 
 extern void initialize_utils (void);
Index: main.c
===================================================================
RCS file: /cvs/src/src/gdb/main.c,v
retrieving revision 1.93
diff -u -p -d -u -r1.93 main.c
--- main.c	7 Mar 2011 18:34:31 -0000	1.93
+++ main.c	9 May 2011 19:49:47 -0000
@@ -104,6 +104,7 @@ extern char *external_editor_command;
    file or directory.  FLAG is true if the value is relocatable, false
    otherwise.  Returns a newly allocated string; this may return NULL
    under the same conditions as make_relative_prefix.  */
+
 static char *
 relocate_path (const char *progname, const char *initial, int flag)
 {
@@ -117,12 +118,13 @@ relocate_path (const char *progname, con
    the result is a directory, it is used; otherwise, INITIAL is used.
    The chosen directory is then canonicalized using lrealpath.  This
    function always returns a newly-allocated string.  */
-static char *
-relocate_directory (const char *progname, const char *initial, int flag)
+
+char *
+relocate_gdb_directory (const char *initial, int flag)
 {
   char *dir;
 
-  dir = relocate_path (progname, initial, flag);
+  dir = relocate_path (gdb_program_name, initial, flag);
   if (dir)
     {
       struct stat s;
@@ -342,22 +344,21 @@ captured_main (void *data)
   current_directory = gdb_dirbuf;
 
   /* Set the sysroot path.  */
-  gdb_sysroot = relocate_directory (argv[0], TARGET_SYSTEM_ROOT,
-				    TARGET_SYSTEM_ROOT_RELOCATABLE);
+  gdb_sysroot = relocate_gdb_directory (TARGET_SYSTEM_ROOT,
+					TARGET_SYSTEM_ROOT_RELOCATABLE);
 
-  debug_file_directory = relocate_directory (argv[0], DEBUGDIR,
-					     DEBUGDIR_RELOCATABLE);
+  debug_file_directory = relocate_gdb_directory (DEBUGDIR,
+						 DEBUGDIR_RELOCATABLE);
 
-  gdb_datadir = relocate_directory (argv[0], GDB_DATADIR,
-				    GDB_DATADIR_RELOCATABLE);
+  gdb_datadir = relocate_gdb_directory (GDB_DATADIR,
+					GDB_DATADIR_RELOCATABLE);
 
 #ifdef WITH_PYTHON_PATH
   {
     /* For later use in helping Python find itself.  */
     char *tmp = concat (WITH_PYTHON_PATH, SLASH_STRING, "lib", NULL);
 
-    python_libdir = relocate_directory (argv[0], tmp,
-					PYTHON_PATH_RELOCATABLE);
+    python_libdir = relocate_gdb_directory (tmp, PYTHON_PATH_RELOCATABLE);
     xfree (tmp);
   }
 #endif
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.830
diff -u -p -d -u -r1.830 gdb.texinfo
--- doc/gdb.texinfo	6 May 2011 18:46:33 -0000	1.830
+++ doc/gdb.texinfo	9 May 2011 19:49:47 -0000
@@ -31214,7 +31214,12 @@ Sets}) require a functioning @code{iconv
 on a GNU system, then this is provided by the GNU C Library.  Some
 other systems also provide a working @code{iconv}.
 
-On systems with @code{iconv}, you can install GNU Libiconv.  If you
+If @value{GDBN} is using the @code{iconv} program and it is installed in
+a non-standard place, you will need to tell @value{GDBN} where to find it.
+This is done with @option{--with-iconv-bin} which specifies the
+directory that contains the @code{iconv} program.
+
+On systems without @code{iconv}, you can install GNU Libiconv.  If you
 have previously installed Libiconv, you can use the
 @option{--with-libiconv-prefix} option to configure.
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [doc RFA]: New option --with-iconv-bin
       [not found]   ` <BANLkTikvtS3eSk_8e+40X5ky9h_HEX5VNQ@mail.gmail.com>
@ 2011-05-09 20:36     ` Eli Zaretskii
  2011-05-09 21:08       ` Doug Evans
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2011-05-09 20:36 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

> Date: Mon, 9 May 2011 13:19:56 -0700
> From: Doug Evans <dje@google.com>
> Cc: gdb-patches@sourceware.org
> 
> > Why not just say "if your @command{iconv} program is installed in a
> > non-standard place, you will need to tell @value{GDBN} where to find
> > it."?
> >
> 
> The iconv program comes from glibc, but you're right, it doesn't
> *necessarily* come from there.
> 
> How about this?

It's fine, but...

> +If @value{GDBN} is using the @code{iconv} program and it is installed in
> +a non-standard place, you will need to tell @value{GDBN} where to find it.

The first "it" is ambiguous: it could have meant GDB or iconv.
Suggest a slight rephrase:

 If @value{GDBN} is using the @code{iconv} program which is installed
 in a non-standard place, ...


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [doc RFA]: New option --with-iconv-bin
  2011-05-09 20:36     ` Eli Zaretskii
@ 2011-05-09 21:08       ` Doug Evans
  2011-05-09 21:53         ` Doug Evans
  0 siblings, 1 reply; 7+ messages in thread
From: Doug Evans @ 2011-05-09 21:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On Mon, May 9, 2011 at 1:36 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Mon, 9 May 2011 13:19:56 -0700
>> From: Doug Evans <dje@google.com>
>> Cc: gdb-patches@sourceware.org
>>
>> > Why not just say "if your @command{iconv} program is installed in a
>> > non-standard place, you will need to tell @value{GDBN} where to find
>> > it."?
>> >
>>
>> The iconv program comes from glibc, but you're right, it doesn't
>> *necessarily* come from there.
>>
>> How about this?
>
> It's fine, but...
>
>> +If @value{GDBN} is using the @code{iconv} program and it is installed in
>> +a non-standard place, you will need to tell @value{GDBN} where to find it.
>
> The first "it" is ambiguous: it could have meant GDB or iconv.

Yeah, I recognized the ambiguity in earlier versions and explicitly
tried a couple of variations to avoid the ambiguity.
I like what I came up with but it can always be improved of course.

> Suggest a slight rephrase:
>
>  If @value{GDBN} is using the @code{iconv} program which is installed
>  in a non-standard place, ...

Ok.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [doc RFA]: New option --with-iconv-bin
  2011-05-09 21:08       ` Doug Evans
@ 2011-05-09 21:53         ` Doug Evans
  0 siblings, 0 replies; 7+ messages in thread
From: Doug Evans @ 2011-05-09 21:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]

On Mon, May 9, 2011 at 2:08 PM, Doug Evans <dje@google.com> wrote:
> On Mon, May 9, 2011 at 1:36 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> Date: Mon, 9 May 2011 13:19:56 -0700
>>> From: Doug Evans <dje@google.com>
>>> Cc: gdb-patches@sourceware.org
>>>
>>> > Why not just say "if your @command{iconv} program is installed in a
>>> > non-standard place, you will need to tell @value{GDBN} where to find
>>> > it."?
>>> >
>>>
>>> The iconv program comes from glibc, but you're right, it doesn't
>>> *necessarily* come from there.
>>>
>>> How about this?
>>
>> It's fine, but...
>>
>>> +If @value{GDBN} is using the @code{iconv} program and it is installed in
>>> +a non-standard place, you will need to tell @value{GDBN} where to find it.
>>
>> The first "it" is ambiguous: it could have meant GDB or iconv.
>
> Yeah, I recognized the ambiguity in earlier versions and explicitly
> tried a couple of variations to avoid the ambiguity.
> I like what I came up with but it can always be improved of course.
>
>> Suggest a slight rephrase:
>>
>>  If @value{GDBN} is using the @code{iconv} program which is installed
>>  in a non-standard place, ...
>
> Ok.
>

Here is the patch I checked in.

[-- Attachment #2: gdb-110509-with-iconv-bin-3.patch.txt --]
[-- Type: text/plain, Size: 8586 bytes --]

2011-05-09  Doug Evans  <dje@google.com>

	* NEWS: Mention --with-iconv-bin.
	* configure.ac: New option --with-iconv-bin.
	* configure: Regenerate.
	* config.in: Regenerate.
	* defs.h (relocate_gdb_directory): Declare.
	* main.c (relocate_gdb_directory): Renamed from relocate_directory,
	removed progname parameter, and exported.  All callers updated.
	* charset.c (find_charset_names): Use --with-iconv-bin if specified.

	doc/
	* gdb.texinfo (Requirements): Fix typo.  Mention --with-iconv-bin.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.437
diff -u -p -d -u -r1.437 NEWS
--- NEWS	6 May 2011 18:46:31 -0000	1.437
+++ NEWS	9 May 2011 21:43:52 -0000
@@ -3,6 +3,12 @@
 
 *** Changes since GDB 7.3
 
+* New configure option --with-iconv-bin.
+  When using the internationalization support like the one in the GNU C
+  library, GDB will invoke the "iconv" program to get a list of supported
+  character sets.  If this program lives in a non-standard location, one can
+  use this option to specify where to find it.
+
 * When natively debugging programs on PowerPC BookE processors running
   a Linux kernel version 2.6.34 or later, GDB supports masked hardware
   watchpoints, which specify a mask in addition to an address to watch.
Index: charset.c
===================================================================
RCS file: /cvs/src/src/gdb/charset.c,v
retrieving revision 1.44
diff -u -p -d -u -r1.44 charset.c
--- charset.c	21 Apr 2011 14:26:38 -0000	1.44
+++ charset.c	9 May 2011 21:43:52 -0000
@@ -799,7 +799,9 @@ find_charset_names (void)
   char *args[3];
   int err, status;
   int fail = 1;
+  int flags;
   struct gdb_environ *iconv_env;
+  char *iconv_program;
 
   /* 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
@@ -811,12 +813,26 @@ find_charset_names (void)
 
   child = pex_init (PEX_USE_PIPES, "iconv", NULL);
 
-  args[0] = "iconv";
+#ifdef ICONV_BIN
+  {
+    char *iconv_dir = relocate_gdb_directory (ICONV_BIN,
+					      ICONV_BIN_RELOCATABLE);
+    iconv_program = concat (iconv_dir, SLASH_STRING, "iconv", NULL);
+    xfree (iconv_dir);
+  }
+#else
+  iconv_program = xstrdup ("iconv");
+#endif
+  args[0] = iconv_program;
   args[1] = "-l";
   args[2] = NULL;
+  flags = PEX_STDERR_TO_STDOUT;
+#ifndef ICONV_BIN
+  flags |= PEX_SEARCH;
+#endif
   /* Note that we simply ignore errors here.  */
-  if (!pex_run_in_environment (child, PEX_SEARCH | PEX_STDERR_TO_STDOUT,
-			       "iconv", args, environ_vector (iconv_env),
+  if (!pex_run_in_environment (child, flags,
+			       args[0], args, environ_vector (iconv_env),
 			       NULL, NULL, &err))
     {
       FILE *in = pex_read_output (child, 0);
@@ -888,6 +904,7 @@ find_charset_names (void)
 
     }
 
+  xfree (iconv_program);
   pex_free (child);
   free_environ (iconv_env);
 
Index: configure.ac
===================================================================
RCS file: /cvs/src/src/gdb/configure.ac,v
retrieving revision 1.144
diff -u -p -d -u -r1.144 configure.ac
--- configure.ac	17 Mar 2011 13:19:10 -0000	1.144
+++ configure.ac	9 May 2011 21:43:53 -0000
@@ -433,6 +433,29 @@ AC_SEARCH_LIBS(dlgetmodinfo, [dl xpdl])
 
 AM_ICONV
 
+# GDB may fork/exec the iconv program to get the list of supported character
+# sets.  Allow the user to specify where to find it.
+# There are several factors affecting the choice of option name:
+# - There is already --with-libiconv-prefix but we can't use it, it specifies
+#   the build-time location of libiconv files.
+# - The program we need to find is iconv, which comes with glibc.  The user
+#   doesn't necessarily have libiconv installed.  Therefore naming this
+#   --with-libiconv-foo feels wrong.
+# - We want the path to be relocatable, but GDB_AC_DEFINE_RELOCATABLE is
+#   defined to work on directories not files (though it really doesn't know
+#   the difference).
+# - Calling this --with-iconv-prefix is perceived to cause too much confusion
+#   with --with-libiconv-prefix.
+# Putting these together is why the option name is --with-iconv-bin.
+
+AC_ARG_WITH(iconv-bin,
+AS_HELP_STRING([--with-iconv-bin=PATH], [specify where to find the iconv program]),
+[iconv_bin="${withval}"
+ AC_DEFINE_UNQUOTED([ICONV_BIN], ["${iconv_bin}"],
+                    [Path of directory of iconv program.])
+ GDB_AC_DEFINE_RELOCATABLE(ICONV_BIN, iconv, ${iconv_bin})
+])
+
 # On alpha-osf, it appears that libtermcap and libcurses are not compatible.
 # There is a very specific comment in /usr/include/curses.h explaining that
 # termcap routines built into libcurses must not be used.
Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.291
diff -u -p -d -u -r1.291 defs.h
--- defs.h	25 Apr 2011 18:28:52 -0000	1.291
+++ defs.h	9 May 2011 21:43:53 -0000
@@ -282,6 +282,12 @@ struct breakpoint;
 struct frame_info;
 struct gdbarch;
 
+/* From main.c.  */
+
+/* This really belong in utils.c (path-utils.c?), but it references some
+   globals that are currently only available to main.c.  */
+extern char *relocate_gdb_directory (const char *initial, int flag);
+
 /* From utils.c */
 
 extern void initialize_utils (void);
Index: main.c
===================================================================
RCS file: /cvs/src/src/gdb/main.c,v
retrieving revision 1.93
diff -u -p -d -u -r1.93 main.c
--- main.c	7 Mar 2011 18:34:31 -0000	1.93
+++ main.c	9 May 2011 21:43:53 -0000
@@ -104,6 +104,7 @@ extern char *external_editor_command;
    file or directory.  FLAG is true if the value is relocatable, false
    otherwise.  Returns a newly allocated string; this may return NULL
    under the same conditions as make_relative_prefix.  */
+
 static char *
 relocate_path (const char *progname, const char *initial, int flag)
 {
@@ -117,12 +118,13 @@ relocate_path (const char *progname, con
    the result is a directory, it is used; otherwise, INITIAL is used.
    The chosen directory is then canonicalized using lrealpath.  This
    function always returns a newly-allocated string.  */
-static char *
-relocate_directory (const char *progname, const char *initial, int flag)
+
+char *
+relocate_gdb_directory (const char *initial, int flag)
 {
   char *dir;
 
-  dir = relocate_path (progname, initial, flag);
+  dir = relocate_path (gdb_program_name, initial, flag);
   if (dir)
     {
       struct stat s;
@@ -342,22 +344,21 @@ captured_main (void *data)
   current_directory = gdb_dirbuf;
 
   /* Set the sysroot path.  */
-  gdb_sysroot = relocate_directory (argv[0], TARGET_SYSTEM_ROOT,
-				    TARGET_SYSTEM_ROOT_RELOCATABLE);
+  gdb_sysroot = relocate_gdb_directory (TARGET_SYSTEM_ROOT,
+					TARGET_SYSTEM_ROOT_RELOCATABLE);
 
-  debug_file_directory = relocate_directory (argv[0], DEBUGDIR,
-					     DEBUGDIR_RELOCATABLE);
+  debug_file_directory = relocate_gdb_directory (DEBUGDIR,
+						 DEBUGDIR_RELOCATABLE);
 
-  gdb_datadir = relocate_directory (argv[0], GDB_DATADIR,
-				    GDB_DATADIR_RELOCATABLE);
+  gdb_datadir = relocate_gdb_directory (GDB_DATADIR,
+					GDB_DATADIR_RELOCATABLE);
 
 #ifdef WITH_PYTHON_PATH
   {
     /* For later use in helping Python find itself.  */
     char *tmp = concat (WITH_PYTHON_PATH, SLASH_STRING, "lib", NULL);
 
-    python_libdir = relocate_directory (argv[0], tmp,
-					PYTHON_PATH_RELOCATABLE);
+    python_libdir = relocate_gdb_directory (tmp, PYTHON_PATH_RELOCATABLE);
     xfree (tmp);
   }
 #endif
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.830
diff -u -p -d -u -r1.830 gdb.texinfo
--- doc/gdb.texinfo	6 May 2011 18:46:33 -0000	1.830
+++ doc/gdb.texinfo	9 May 2011 21:43:53 -0000
@@ -31214,7 +31214,12 @@ Sets}) require a functioning @code{iconv
 on a GNU system, then this is provided by the GNU C Library.  Some
 other systems also provide a working @code{iconv}.
 
-On systems with @code{iconv}, you can install GNU Libiconv.  If you
+If @value{GDBN} is using the @code{iconv} program which is installed
+in a non-standard place, you will need to tell @value{GDBN} where to find it.
+This is done with @option{--with-iconv-bin} which specifies the
+directory that contains the @code{iconv} program.
+
+On systems without @code{iconv}, you can install GNU Libiconv.  If you
 have previously installed Libiconv, you can use the
 @option{--with-libiconv-prefix} option to configure.
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2011-05-09 21:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-09 18:20 [doc RFA]: New option --with-iconv-bin Doug Evans
2011-05-09 19:09 ` Eli Zaretskii
2011-05-09 20:34   ` Doug Evans
     [not found]   ` <BANLkTikvtS3eSk_8e+40X5ky9h_HEX5VNQ@mail.gmail.com>
2011-05-09 20:36     ` Eli Zaretskii
2011-05-09 21:08       ` Doug Evans
2011-05-09 21:53         ` Doug Evans
2011-05-09 19:41 ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox