Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
@ 2010-01-14  2:44 Hui Zhu
  2010-01-14 10:15 ` Jan Kratochvil
  0 siblings, 1 reply; 16+ messages in thread
From: Hui Zhu @ 2010-01-14  2:44 UTC (permalink / raw)
  To: gdb-patches ml; +Cc: Tristan Gingold, Daniel Jacobowitz, Jan Kratochvil

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

Hello guys,

This patch is part of patch
http://sourceware.org/ml/gdb-patches/2010-01/msg00327.html

I removed the cmd "set bfd".  Keep the tips:
"/ls":Matching formats: elf32-nbigmips elf32-ntradbigmips
Use command "set gnutarget" handle it.

Please help me review it.

Thanks,
Hui

2010-01-14  Hui Zhu  <teawater@gmail.com>

	* exec.c (exec_file_attach): Output some tips when
	bfd_get_error is bfd_error_file_ambiguously_recognized.

[-- Attachment #2: tips.txt --]
[-- Type: text/plain, Size: 1548 bytes --]

---
 exec.c |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

--- a/exec.c
+++ b/exec.c
@@ -207,6 +207,8 @@ exec_file_clear (int from_tty)
 void
 exec_file_attach (char *filename, int from_tty)
 {
+  char **matching;
+
   /* Remove any previous exec file.  */
   exec_close ();
 
@@ -259,13 +261,26 @@ exec_file_attach (char *filename, int fr
       scratch_pathname = xstrdup (scratch_pathname);
       cleanups = make_cleanup (xfree, scratch_pathname);
 
-      if (!bfd_check_format (exec_bfd, bfd_object))
+      if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
 	{
 	  /* Make sure to close exec_bfd, or else "run" might try to use
 	     it.  */
 	  exec_close ();
-	  error (_("\"%s\": not in executable format: %s"),
-		 scratch_pathname, bfd_errmsg (bfd_get_error ()));
+
+          if (bfd_get_error () == bfd_error_file_ambiguously_recognized)
+            {
+              char **p = matching;
+              fprintf_filtered (gdb_stderr, _("\"%s\":Matching formats:"),
+                                scratch_pathname);
+              while (*p)
+                fprintf_filtered (gdb_stderr, " %s", *p++);
+              fprintf_filtered (gdb_stderr, "\n");
+              free (matching);
+              error (_("Use command \"set gnutarget\" handle it."));
+            }
+          else
+	    error (_("\"%s\": not in executable format: %s"),
+		   scratch_pathname, bfd_errmsg (bfd_get_error ()));
 	}
 
       /* FIXME - This should only be run for RS6000, but the ifdef is a poor

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

* Re: [RFA] Show some tips when file cmd get  bfd_error_file_ambiguously_recognized
  2010-01-14  2:44 [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized Hui Zhu
@ 2010-01-14 10:15 ` Jan Kratochvil
  2010-01-19  7:01   ` Hui Zhu
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Kratochvil @ 2010-01-14 10:15 UTC (permalink / raw)
  To: Hui Zhu; +Cc: gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

On Thu, 14 Jan 2010 03:43:58 +0100, Hui Zhu wrote:
>  exec_file_attach (char *filename, int from_tty)
>  {
> +  char **matching;

This new variable could be moved to one more inner block.

> +          if (bfd_get_error () == bfd_error_file_ambiguously_recognized)
> +            {
> +              char **p = matching;

There is commonly an empty line between declarations and first code line.

> +              fprintf_filtered (gdb_stderr, _("\"%s\":Matching formats:"),
> +                                scratch_pathname);

Missing space before "Matching formats".

Isn't it "Matching targets" instead?

Anyway I would still prefer to print even bfd_errmsg() for it as it is now IMO
unclear why "Matching formats" has been printed and that any error occured:

> "/ls":Matching formats: elf32-nbigmips elf32-ntradbigmips
> Use command "set gnutarget" handle it.


> +              while (*p)
> +                fprintf_filtered (gdb_stderr, " %s", *p++);

(Only a personal preference: for (p = matching; *p != NULL; p++)
 as this is the most common iterating loop, nothing special to use `while'.)

> +              fprintf_filtered (gdb_stderr, "\n");

(There exists also fputc_filtered.)

> +              free (matching);

Use xfree (see gdbint); despite the memory does not come from xmalloc.

> +              error (_("Use command \"set gnutarget\" handle it."));

Missing "to" - "to handle it"; or just use _("Use command \"...\".").

> +            }
> +          else
> +	    error (_("\"%s\": not in executable format: %s"),
> +		   scratch_pathname, bfd_errmsg (bfd_get_error ()));
>  	}
>  
>        /* FIXME - This should only be run for RS6000, but the ifdef is a poor

Your patch has been sent with spaces (' ') instead of tabs ('\t') to follow
the GNU coding style (not using `indent -nut').


Regards,
Jan


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

* Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
  2010-01-14 10:15 ` Jan Kratochvil
@ 2010-01-19  7:01   ` Hui Zhu
  2010-01-19  7:57     ` Jan Kratochvil
  0 siblings, 1 reply; 16+ messages in thread
From: Hui Zhu @ 2010-01-19  7:01 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

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

Thanks Jan.

I make a new patch according to your mail.
And keep the "formats".  Because in before, we use "not in executable
format".  And in other program like nm.c size.c, they all use
"format".  So I think format is better.

Hui

2010-01-19  Hui Zhu  <teawater@gmail.com>

	* exec.c (exec_file_attach): Output some tips when
	bfd_get_error is bfd_error_file_ambiguously_recognized.

On Thu, Jan 14, 2010 at 18:13, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> On Thu, 14 Jan 2010 03:43:58 +0100, Hui Zhu wrote:
>>  exec_file_attach (char *filename, int from_tty)
>>  {
>> +  char **matching;
>
> This new variable could be moved to one more inner block.
>
>> +          if (bfd_get_error () == bfd_error_file_ambiguously_recognized)
>> +            {
>> +              char **p = matching;
>
> There is commonly an empty line between declarations and first code line.
>
>> +              fprintf_filtered (gdb_stderr, _("\"%s\":Matching formats:"),
>> +                                scratch_pathname);
>
> Missing space before "Matching formats".
>
> Isn't it "Matching targets" instead?
>
> Anyway I would still prefer to print even bfd_errmsg() for it as it is now IMO
> unclear why "Matching formats" has been printed and that any error occured:
>
>> "/ls":Matching formats: elf32-nbigmips elf32-ntradbigmips
>> Use command "set gnutarget" handle it.
>
>
>> +              while (*p)
>> +                fprintf_filtered (gdb_stderr, " %s", *p++);
>
> (Only a personal preference: for (p = matching; *p != NULL; p++)
>  as this is the most common iterating loop, nothing special to use `while'.)
>
>> +              fprintf_filtered (gdb_stderr, "\n");
>
> (There exists also fputc_filtered.)
>
>> +              free (matching);
>
> Use xfree (see gdbint); despite the memory does not come from xmalloc.
>
>> +              error (_("Use command \"set gnutarget\" handle it."));
>
> Missing "to" - "to handle it"; or just use _("Use command \"...\".").
>
>> +            }
>> +          else
>> +         error (_("\"%s\": not in executable format: %s"),
>> +                scratch_pathname, bfd_errmsg (bfd_get_error ()));
>>       }
>>
>>        /* FIXME - This should only be run for RS6000, but the ifdef is a poor
>
> Your patch has been sent with spaces (' ') instead of tabs ('\t') to follow
> the GNU coding style (not using `indent -nut').
>
>
> Regards,
> Jan
>

[-- Attachment #2: tips.txt --]
[-- Type: text/plain, Size: 1736 bytes --]

---
 exec.c |   22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

--- a/exec.c
+++ b/exec.c
@@ -225,6 +225,7 @@ exec_file_attach (char *filename, int fr
       char *scratch_pathname;
       int scratch_chan;
       struct target_section *sections = NULL, *sections_end = NULL;
+      char **matching;
 
       scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, filename,
 		   write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
@@ -259,13 +260,28 @@ exec_file_attach (char *filename, int fr
       scratch_pathname = xstrdup (scratch_pathname);
       cleanups = make_cleanup (xfree, scratch_pathname);
 
-      if (!bfd_check_format (exec_bfd, bfd_object))
+      if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
 	{
 	  /* Make sure to close exec_bfd, or else "run" might try to use
 	     it.  */
 	  exec_close ();
-	  error (_("\"%s\": not in executable format: %s"),
-		 scratch_pathname, bfd_errmsg (bfd_get_error ()));
+
+	  if (bfd_get_error () == bfd_error_file_ambiguously_recognized)
+	    {
+	      char **p;
+
+	      fprintf_filtered (gdb_stderr, _("\"%s\": not in executable format: %s\n"),
+		                scratch_pathname, bfd_errmsg (bfd_get_error ()));
+	      fprintf_filtered (gdb_stderr, _("It matchs formats:"));
+	      for (p = matching; *p; p++)
+		fprintf_filtered (gdb_stderr, " %s", *p);
+	      fputc_filtered ('\n', gdb_stderr);
+	      xfree (matching);
+	      error (_("Use command \"set gnutarget format_name\" to handle it."));
+	    }
+	  else
+	    error (_("\"%s\": not in executable format: %s"),
+		   scratch_pathname, bfd_errmsg (bfd_get_error ()));
 	}
 
       /* FIXME - This should only be run for RS6000, but the ifdef is a poor

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

* Re: [RFA] Show some tips when file cmd get  bfd_error_file_ambiguously_recognized
  2010-01-19  7:01   ` Hui Zhu
@ 2010-01-19  7:57     ` Jan Kratochvil
  2010-01-19  8:08       ` Hui Zhu
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Kratochvil @ 2010-01-19  7:57 UTC (permalink / raw)
  To: Hui Zhu; +Cc: gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

On Tue, 19 Jan 2010 08:01:18 +0100, Hui Zhu wrote:
> And keep the "formats".  Because in before, we use "not in executable
> format".  And in other program like nm.c size.c, they all use
> "format".  So I think format is better.

OK, I see it is used more now.


> +	      fprintf_filtered (gdb_stderr, _("It matchs formats:"));

"matches"


Thanks,
Jan


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

* Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
  2010-01-19  7:57     ` Jan Kratochvil
@ 2010-01-19  8:08       ` Hui Zhu
  2010-01-26  8:01         ` Hui Zhu
  0 siblings, 1 reply; 16+ messages in thread
From: Hui Zhu @ 2010-01-19  8:08 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

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

>
>> +           fprintf_filtered (gdb_stderr, _("It matchs formats:"));
>
> "matches"
>

Fixed.

Thanks,
Hui

2010-01-19  Hui Zhu  <teawater@gmail.com>

	* exec.c (exec_file_attach): Output some tips when
	bfd_get_error is bfd_error_file_ambiguously_recognized.

[-- Attachment #2: tips.txt --]
[-- Type: text/plain, Size: 1737 bytes --]

---
 exec.c |   22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

--- a/exec.c
+++ b/exec.c
@@ -225,6 +225,7 @@ exec_file_attach (char *filename, int fr
       char *scratch_pathname;
       int scratch_chan;
       struct target_section *sections = NULL, *sections_end = NULL;
+      char **matching;
 
       scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, filename,
 		   write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
@@ -259,13 +260,28 @@ exec_file_attach (char *filename, int fr
       scratch_pathname = xstrdup (scratch_pathname);
       cleanups = make_cleanup (xfree, scratch_pathname);
 
-      if (!bfd_check_format (exec_bfd, bfd_object))
+      if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
 	{
 	  /* Make sure to close exec_bfd, or else "run" might try to use
 	     it.  */
 	  exec_close ();
-	  error (_("\"%s\": not in executable format: %s"),
-		 scratch_pathname, bfd_errmsg (bfd_get_error ()));
+
+	  if (bfd_get_error () == bfd_error_file_ambiguously_recognized)
+	    {
+	      char **p;
+
+	      fprintf_filtered (gdb_stderr, _("\"%s\": not in executable format: %s\n"),
+		                scratch_pathname, bfd_errmsg (bfd_get_error ()));
+	      fprintf_filtered (gdb_stderr, _("It matches formats:"));
+	      for (p = matching; *p; p++)
+		fprintf_filtered (gdb_stderr, " %s", *p);
+	      fputc_filtered ('\n', gdb_stderr);
+	      xfree (matching);
+	      error (_("Use command \"set gnutarget format_name\" to handle it."));
+	    }
+	  else
+	    error (_("\"%s\": not in executable format: %s"),
+		   scratch_pathname, bfd_errmsg (bfd_get_error ()));
 	}
 
       /* FIXME - This should only be run for RS6000, but the ifdef is a poor

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

* Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
  2010-01-19  8:08       ` Hui Zhu
@ 2010-01-26  8:01         ` Hui Zhu
  2010-01-26 21:04           ` Doug Evans
  0 siblings, 1 reply; 16+ messages in thread
From: Hui Zhu @ 2010-01-26  8:01 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

About this patch.  There are a lot of function call "bfd_check_format".

What about add a wrapper of "bfd_check_format_matches" to gdb.  This
wrapper call "bfd_check_format_matches" and if it get
bfd_error_file_ambiguously_recognized, output the format and other
help message before it return.
Change the "bfd_check_format" to the wrapper.

Then when gdb get bfd_error_file_ambiguously_recognized, putput the
help message directly.

What do you think about it?

Thanks,
Hui

On Tue, Jan 19, 2010 at 16:08, Hui Zhu <teawater@gmail.com> wrote:
>>
>>> +           fprintf_filtered (gdb_stderr, _("It matchs formats:"));
>>
>> "matches"
>>
>
> Fixed.
>
> Thanks,
> Hui
>
> 2010-01-19  Hui Zhu  <teawater@gmail.com>
>
>        * exec.c (exec_file_attach): Output some tips when
>        bfd_get_error is bfd_error_file_ambiguously_recognized.
>


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

* Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
  2010-01-26  8:01         ` Hui Zhu
@ 2010-01-26 21:04           ` Doug Evans
  2010-01-27  9:01             ` Hui Zhu
  0 siblings, 1 reply; 16+ messages in thread
From: Doug Evans @ 2010-01-26 21:04 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Jan Kratochvil, gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

On Tue, Jan 26, 2010 at 12:01 AM, Hui Zhu <teawater@gmail.com> wrote:
> About this patch.  There are a lot of function call "bfd_check_format".
>
> What about add a wrapper of "bfd_check_format_matches" to gdb.  This
> wrapper call "bfd_check_format_matches" and if it get
> bfd_error_file_ambiguously_recognized, output the format and other
> help message before it return.
> Change the "bfd_check_format" to the wrapper.
>
> Then when gdb get bfd_error_file_ambiguously_recognized, putput the
> help message directly.
>
> What do you think about it?

AIUC, At first glance I don't like it.
"Detect errors at a low level, handle them at a high level."
[ref: "The Practice of Programming"]
Replacing all calls to bfd_check_format with something that prints
anything is the wrong way to go (bfd_check_format, or its wrapper is
too low a level).

Unless by "output the format and other help message before it return"
you mean to just compute the message and leave it to some higher level
caller to print it.  If that's the case I think I'd just add a new
function to compute the error message for the error printer to use.


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

* Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
  2010-01-26 21:04           ` Doug Evans
@ 2010-01-27  9:01             ` Hui Zhu
  2010-01-28 18:18               ` Doug Evans
  2010-01-28 21:56               ` Jan Kratochvil
  0 siblings, 2 replies; 16+ messages in thread
From: Hui Zhu @ 2010-01-27  9:01 UTC (permalink / raw)
  To: Doug Evans
  Cc: Jan Kratochvil, gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

On Wed, Jan 27, 2010 at 05:04, Doug Evans <dje@google.com> wrote:
> On Tue, Jan 26, 2010 at 12:01 AM, Hui Zhu <teawater@gmail.com> wrote:
>> About this patch.  There are a lot of function call "bfd_check_format".
>>
>> What about add a wrapper of "bfd_check_format_matches" to gdb.  This
>> wrapper call "bfd_check_format_matches" and if it get
>> bfd_error_file_ambiguously_recognized, output the format and other
>> help message before it return.
>> Change the "bfd_check_format" to the wrapper.
>>
>> Then when gdb get bfd_error_file_ambiguously_recognized, putput the
>> help message directly.
>>
>> What do you think about it?
>
> AIUC, At first glance I don't like it.
> "Detect errors at a low level, handle them at a high level."
> [ref: "The Practice of Programming"]
> Replacing all calls to bfd_check_format with something that prints
> anything is the wrong way to go (bfd_check_format, or its wrapper is
> too low a level).
>
> Unless by "output the format and other help message before it return"
> you mean to just compute the message and leave it to some higher level
> caller to print it.  If that's the case I think I'd just add a new
> function to compute the error message for the error printer to use.
>

Agree with you.  I make a new patch to add a wrapper to bfd_errmsg.
When the bfd_check_format_matches get
bfd_error_file_ambiguously_recognized, it will change the error
message to:

"/xxx/vmlinux": not in executable format: File format is ambiguous, it
matches formats: elf64-bigmips elf64-tradbigmips, "set gnutarget
format-name" handle it

Please help me review it.

Thanks,
Hui

2010-01-27  Hui Zhu  <teawater@gmail.com>

	* defs.h (gdb_bfd_errmsg): New extern.
	* exec.c (exec_file_attach): Change bfd_errmsg to
	gdb_bfd_errmsg.
	* utils.c (AMBIGUOUS_MESS1): New macro.
	(AMBIGUOUS_MESS2): New macro.
	(gdb_bfd_errmsg): New function.

---
 defs.h  |    8 ++++++++
 exec.c  |    6 ++++--
 utils.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 2 deletions(-)

--- a/defs.h
+++ b/defs.h
@@ -1226,4 +1226,12 @@ void dummy_obstack_deallocate (void *obj
 extern void initialize_progspace (void);
 extern void initialize_inferiors (void);

+/* The wrapper for the bfd_errmsg.
+   When error_tag is bfd_error_file_ambiguously_recognized and matching
+   is not NULL, this function will return more help message to handle
+   this issue.  */
+
+extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
+
+
 #endif /* #ifndef DEFS_H */
--- a/exec.c
+++ b/exec.c
@@ -219,6 +219,7 @@ exec_file_attach (char *filename, int fr
       char *scratch_pathname;
       int scratch_chan;
       struct target_section *sections = NULL, *sections_end = NULL;
+      char **matching;

       scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, filename,
 		   write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
@@ -253,13 +254,14 @@ exec_file_attach (char *filename, int fr
       scratch_pathname = xstrdup (scratch_pathname);
       cleanups = make_cleanup (xfree, scratch_pathname);

-      if (!bfd_check_format (exec_bfd, bfd_object))
+      if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
 	{
 	  /* Make sure to close exec_bfd, or else "run" might try to use
 	     it.  */
 	  exec_close ();
 	  error (_("\"%s\": not in executable format: %s"),
-		 scratch_pathname, bfd_errmsg (bfd_get_error ()));
+		 scratch_pathname,
+		 gdb_bfd_errmsg (bfd_get_error (), matching));
 	}

       /* FIXME - This should only be run for RS6000, but the ifdef is a poor
--- a/utils.c
+++ b/utils.c
@@ -3608,6 +3608,58 @@ compare_positive_ints (const void *ap, c
   return * (int *) ap - * (int *) bp;
 }

+#define AMBIGUOUS_MESS1	", it matches formats:"
+#define AMBIGUOUS_MESS2	", \"set gnutarget format-name\" handle it"
+
+const char *
+gdb_bfd_errmsg (bfd_error_type error_tag, char **matching)
+{
+  char *ret;
+  int ret_len, current_ret_len = 0;
+  char **p;
+
+  /* Check if errmsg just need simple return.  */
+  if (error_tag != bfd_error_file_ambiguously_recognized || !matching)
+    return bfd_errmsg (error_tag);
+
+  ret_len = strlen (bfd_errmsg (error_tag)) + strlen (AMBIGUOUS_MESS1)
+	    + 50 + strlen (AMBIGUOUS_MESS2);
+  ret = xmalloc (ret_len + 1);
+
+  /* bfd_errmsg (error_tag) */
+  strcpy (ret + current_ret_len, bfd_errmsg (error_tag));
+  current_ret_len += strlen (bfd_errmsg (error_tag));
+
+  /* AMBIGUOUS_MESS1 */
+  strcpy (ret + current_ret_len, AMBIGUOUS_MESS1);
+  current_ret_len += strlen (AMBIGUOUS_MESS1);
+
+  /* matching */
+  for (p = matching; *p; p++)
+    {
+      if (current_ret_len + strlen (*p) + 1 > ret_len)
+        {
+          ret_len += 50;
+          ret = xrealloc (ret, ret_len + 1);
+        }
+      sprintf (ret + current_ret_len, " %s", *p);
+      current_ret_len += strlen (*p) + 1;
+    }
+  xfree (matching);
+
+  /* AMBIGUOUS_MESS2 */
+  if (current_ret_len + strlen (AMBIGUOUS_MESS2) > ret_len)
+    {
+      ret_len += strlen (AMBIGUOUS_MESS2);
+      ret = xrealloc (ret, ret_len + 1);
+    }
+  strcpy (ret + current_ret_len, AMBIGUOUS_MESS2);
+
+  make_cleanup (xfree, ret);
+
+  return ret;
+}
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_utils;


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

* Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
  2010-01-27  9:01             ` Hui Zhu
@ 2010-01-28 18:18               ` Doug Evans
  2010-01-28 21:56               ` Jan Kratochvil
  1 sibling, 0 replies; 16+ messages in thread
From: Doug Evans @ 2010-01-28 18:18 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Jan Kratochvil, gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

On Wed, Jan 27, 2010 at 1:00 AM, Hui Zhu <teawater@gmail.com> wrote:
> Agree with you.  I make a new patch to add a wrapper to bfd_errmsg.
> When the bfd_check_format_matches get
> bfd_error_file_ambiguously_recognized, it will change the error
> message to:
>
> "/xxx/vmlinux": not in executable format: File format is ambiguous, it
> matches formats: elf64-bigmips elf64-tradbigmips, "set gnutarget
> format-name" handle it

I like the patch, but several nits.
First the wording of the error message needs to be improved.
IWBN to spread this over several lines.  There is precedent for it.
How about

"/xxx/vmlinux": not in executable format: File format is ambiguous.
Matching formats: elf64-bigmips, elf64-tradbigmips.
Use "set gnutarget format-name" to specify the format.

More comments inline below.

> Please help me review it.
>
> Thanks,
> Hui
>
> 2010-01-27  Hui Zhu  <teawater@gmail.com>
>
>        * defs.h (gdb_bfd_errmsg): New extern.
>        * exec.c (exec_file_attach): Change bfd_errmsg to
>        gdb_bfd_errmsg.
>        * utils.c (AMBIGUOUS_MESS1): New macro.
>        (AMBIGUOUS_MESS2): New macro.
>        (gdb_bfd_errmsg): New function.
>
> ---
>  defs.h  |    8 ++++++++
>  exec.c  |    6 ++++--
>  utils.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 64 insertions(+), 2 deletions(-)
>
> --- a/defs.h
> +++ b/defs.h
> @@ -1226,4 +1226,12 @@ void dummy_obstack_deallocate (void *obj
>  extern void initialize_progspace (void);
>  extern void initialize_inferiors (void);
>
> +/* The wrapper for the bfd_errmsg.
> +   When error_tag is bfd_error_file_ambiguously_recognized and matching
> +   is not NULL, this function will return more help message to handle
> +   this issue.  */

I'd add a note saying that matching is set by calls to
bfd_check_format_matches and that this function will free it.
Plus I'd rephrase the rest of the comment.

/* A wrapper for bfd_errmsg to produce a more helpful error message
   in the case of bfd_error_file_ambiguously recognized.
   MATCHING, if non-NULL, is the corresponding argument to
bfd_check_format_matches,
   and will be freed.  */

> +
> +extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
> +
> +
>  #endif /* #ifndef DEFS_H */

grep for utils.c in defs.h.  There's a section in defs.h for
prototypes for functions from utils.c

> --- a/exec.c
> +++ b/exec.c
> @@ -219,6 +219,7 @@ exec_file_attach (char *filename, int fr
>       char *scratch_pathname;
>       int scratch_chan;
>       struct target_section *sections = NULL, *sections_end = NULL;
> +      char **matching;
>
>       scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, filename,
>                   write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
> @@ -253,13 +254,14 @@ exec_file_attach (char *filename, int fr
>       scratch_pathname = xstrdup (scratch_pathname);
>       cleanups = make_cleanup (xfree, scratch_pathname);
>
> -      if (!bfd_check_format (exec_bfd, bfd_object))
> +      if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
>        {
>          /* Make sure to close exec_bfd, or else "run" might try to use
>             it.  */
>          exec_close ();
>          error (_("\"%s\": not in executable format: %s"),
> -                scratch_pathname, bfd_errmsg (bfd_get_error ()));
> +                scratch_pathname,
> +                gdb_bfd_errmsg (bfd_get_error (), matching));
>        }
>
>       /* FIXME - This should only be run for RS6000, but the ifdef is a poor
> --- a/utils.c
> +++ b/utils.c
> @@ -3608,6 +3608,58 @@ compare_positive_ints (const void *ap, c
>   return * (int *) ap - * (int *) bp;
>  }
>
> +#define AMBIGUOUS_MESS1        ", it matches formats:"
> +#define AMBIGUOUS_MESS2        ", \"set gnutarget format-name\" handle it"
> +
> +const char *
> +gdb_bfd_errmsg (bfd_error_type error_tag, char **matching)
> +{
> +  char *ret;
> +  int ret_len, current_ret_len = 0;
> +  char **p;
> +
> +  /* Check if errmsg just need simple return.  */
> +  if (error_tag != bfd_error_file_ambiguously_recognized || !matching)
> +    return bfd_errmsg (error_tag);

I believe the convention is to write matching == NULL instead of !matching.

> +
> +  ret_len = strlen (bfd_errmsg (error_tag)) + strlen (AMBIGUOUS_MESS1)
> +           + 50 + strlen (AMBIGUOUS_MESS2);
> +  ret = xmalloc (ret_len + 1);

I would compute the complete length needed, rather than watching for
when to call xrealloc.
That means two iterations over matching, but that's ok.

> +
> +  /* bfd_errmsg (error_tag) */

There's no real need for this comment, it doesn't contribute enough
(IMO anyway).
Ditto for similar comments below.

> +  strcpy (ret + current_ret_len, bfd_errmsg (error_tag));
> +  current_ret_len += strlen (bfd_errmsg (error_tag));

I'd write

  strcpy (p, ...);
  p += strlen (p);

instead, but that's just a suggestion (but please make the other
changes requested in this email though).
[Note: There is stpcpy, but we can't use it.]

> +
> +  /* AMBIGUOUS_MESS1 */
> +  strcpy (ret + current_ret_len, AMBIGUOUS_MESS1);
> +  current_ret_len += strlen (AMBIGUOUS_MESS1);
> +
> +  /* matching */
> +  for (p = matching; *p; p++)
> +    {
> +      if (current_ret_len + strlen (*p) + 1 > ret_len)
> +        {
> +          ret_len += 50;
> +          ret = xrealloc (ret, ret_len + 1);
> +        }
> +      sprintf (ret + current_ret_len, " %s", *p);
> +      current_ret_len += strlen (*p) + 1;
> +    }
> +  xfree (matching);
> +
> +  /* AMBIGUOUS_MESS2 */
> +  if (current_ret_len + strlen (AMBIGUOUS_MESS2) > ret_len)
> +    {
> +      ret_len += strlen (AMBIGUOUS_MESS2);
> +      ret = xrealloc (ret, ret_len + 1);
> +    }
> +  strcpy (ret + current_ret_len, AMBIGUOUS_MESS2);
> +
> +  make_cleanup (xfree, ret);
> +
> +  return ret;
> +}
> +
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
>  extern initialize_file_ftype _initialize_utils;
>


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

* Re: [RFA] Show some tips when file cmd get  bfd_error_file_ambiguously_recognized
  2010-01-27  9:01             ` Hui Zhu
  2010-01-28 18:18               ` Doug Evans
@ 2010-01-28 21:56               ` Jan Kratochvil
  2010-01-28 21:59                 ` Doug Evans
  1 sibling, 1 reply; 16+ messages in thread
From: Jan Kratochvil @ 2010-01-28 21:56 UTC (permalink / raw)
  To: Hui Zhu; +Cc: Doug Evans, gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

On Wed, 27 Jan 2010 10:00:34 +0100, Hui Zhu wrote:
> +  ret_len = strlen (bfd_errmsg (error_tag)) + strlen (AMBIGUOUS_MESS1)
> +	    + 50 + strlen (AMBIGUOUS_MESS2);
> +  ret = xmalloc (ret_len + 1);

Rather than all the C strings manipulation fragile magic maybe one could
afford mem_fileopen with ui_file_xstrdup (as being done sometimes in GDB).

Sure it may have been clear and not chosen as very ineffective.


Regards,
Jan


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

* Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
  2010-01-28 21:56               ` Jan Kratochvil
@ 2010-01-28 21:59                 ` Doug Evans
  2010-01-29  3:51                   ` Joel Brobecker
  0 siblings, 1 reply; 16+ messages in thread
From: Doug Evans @ 2010-01-28 21:59 UTC (permalink / raw)
  To: Jan Kratochvil
  Cc: Hui Zhu, gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

On Thu, Jan 28, 2010 at 1:56 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Wed, 27 Jan 2010 10:00:34 +0100, Hui Zhu wrote:
>> +  ret_len = strlen (bfd_errmsg (error_tag)) + strlen (AMBIGUOUS_MESS1)
>> +         + 50 + strlen (AMBIGUOUS_MESS2);
>> +  ret = xmalloc (ret_len + 1);
>
> Rather than all the C strings manipulation fragile magic maybe one could
> afford mem_fileopen with ui_file_xstrdup (as being done sometimes in GDB).

Fine with me.
[I've always lamented the absence of such a facility in C itself.]


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

* Re: [RFA] Show some tips when file cmd get  bfd_error_file_ambiguously_recognized
  2010-01-28 21:59                 ` Doug Evans
@ 2010-01-29  3:51                   ` Joel Brobecker
  2010-01-29  9:06                     ` Hui Zhu
  0 siblings, 1 reply; 16+ messages in thread
From: Joel Brobecker @ 2010-01-29  3:51 UTC (permalink / raw)
  To: Doug Evans
  Cc: Jan Kratochvil, Hui Zhu, gdb-patches ml, Tristan Gingold,
	Daniel Jacobowitz

> > Rather than all the C strings manipulation fragile magic maybe one could
> > afford mem_fileopen with ui_file_xstrdup (as being done sometimes in GDB).
> 
> Fine with me.
> [I've always lamented the absence of such a facility in C itself.]

I made the very same suggestion when reviewing the patch for D support,
and Tom reminded us of of another option: libiberty/dyn-string.c
(looking at this code, the memory allocated is doubled every time
the buffer needs to be expanded, so it should be relatively efficient).

-- 
Joel


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

* Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
  2010-01-29  3:51                   ` Joel Brobecker
@ 2010-01-29  9:06                     ` Hui Zhu
  2010-02-04  7:09                       ` Hui Zhu
  0 siblings, 1 reply; 16+ messages in thread
From: Hui Zhu @ 2010-01-29  9:06 UTC (permalink / raw)
  To: Joel Brobecker, Doug Evans, Jan Kratochvil
  Cc: gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

Thanks guys.  I make a new patch according to your mails.

And about the xrealloc, I don't know what I was thinking when I use
it.  It is really bad, ugly and wasted me a lot of time.  :(

Please help me review the new patch.  Thanks.

Best regards,
Hui

2010-01-29  Hui Zhu  <teawater@gmail.com>

	* defs.h (gdb_bfd_errmsg): New extern.
	* exec.c (exec_file_attach): Change bfd_errmsg to
	gdb_bfd_errmsg.
	* utils.c (AMBIGUOUS_MESS1): New macro.
	(AMBIGUOUS_MESS2): New macro.
	(gdb_bfd_errmsg): New function.

---
 defs.h  |    8 ++++++++
 exec.c  |    6 ++++--
 utils.c |   40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

--- a/defs.h
+++ b/defs.h
@@ -419,6 +419,13 @@ char **gdb_buildargv (const char *);

 int compare_positive_ints (const void *ap, const void *bp);

+/* A wrapper for bfd_errmsg to produce a more helpful error message
+   in the case of bfd_error_file_ambiguously recognized.
+   MATCHING, if non-NULL, is the corresponding argument to
+   bfd_check_format_matches, and will be freed.  */
+
+extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
+
 /* From demangle.c */

 extern void set_demangling_style (char *);
@@ -1226,4 +1233,5 @@ void dummy_obstack_deallocate (void *obj
 extern void initialize_progspace (void);
 extern void initialize_inferiors (void);

+
 #endif /* #ifndef DEFS_H */
--- a/exec.c
+++ b/exec.c
@@ -219,6 +219,7 @@ exec_file_attach (char *filename, int fr
       char *scratch_pathname;
       int scratch_chan;
       struct target_section *sections = NULL, *sections_end = NULL;
+      char **matching;

       scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, filename,
 		   write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
@@ -253,13 +254,14 @@ exec_file_attach (char *filename, int fr
       scratch_pathname = xstrdup (scratch_pathname);
       cleanups = make_cleanup (xfree, scratch_pathname);

-      if (!bfd_check_format (exec_bfd, bfd_object))
+      if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
 	{
 	  /* Make sure to close exec_bfd, or else "run" might try to use
 	     it.  */
 	  exec_close ();
 	  error (_("\"%s\": not in executable format: %s"),
-		 scratch_pathname, bfd_errmsg (bfd_get_error ()));
+		 scratch_pathname,
+		 gdb_bfd_errmsg (bfd_get_error (), matching));
 	}

       /* FIXME - This should only be run for RS6000, but the ifdef is a poor
--- a/utils.c
+++ b/utils.c
@@ -3608,6 +3608,46 @@ compare_positive_ints (const void *ap, c
   return * (int *) ap - * (int *) bp;
 }

+#define AMBIGUOUS_MESS1	".\nMatching formats:"
+#define AMBIGUOUS_MESS2	".\nUse \"set gnutarget format-name\" specify
the format."
+
+const char *
+gdb_bfd_errmsg (bfd_error_type error_tag, char **matching)
+{
+  char *ret, *retp;
+  int ret_len;
+  char **p;
+
+  /* Check if errmsg just need simple return.  */
+  if (error_tag != bfd_error_file_ambiguously_recognized || matching == NULL)
+    return bfd_errmsg (error_tag);
+
+  ret_len = strlen (bfd_errmsg (error_tag)) + strlen (AMBIGUOUS_MESS1)
+            + strlen (AMBIGUOUS_MESS2);
+  for (p = matching; *p; p++)
+    ret_len += strlen (*p) + 1;
+  ret = xmalloc (ret_len + 1);
+  retp = ret;
+  make_cleanup (xfree, ret);
+
+  strcpy (retp, bfd_errmsg (error_tag));
+  retp += strlen (retp);
+
+  strcpy (retp, AMBIGUOUS_MESS1);
+  retp += strlen (retp);
+
+  for (p = matching; *p; p++)
+    {
+      sprintf (retp, " %s", *p);
+      retp += strlen (retp);
+    }
+  xfree (matching);
+
+  strcpy (retp, AMBIGUOUS_MESS2);
+
+  return ret;
+}
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_utils;


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

* Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
  2010-01-29  9:06                     ` Hui Zhu
@ 2010-02-04  7:09                       ` Hui Zhu
  2010-02-04 17:38                         ` Doug Evans
  0 siblings, 1 reply; 16+ messages in thread
From: Hui Zhu @ 2010-02-04  7:09 UTC (permalink / raw)
  To: Joel Brobecker, Doug Evans, Jan Kratochvil
  Cc: gdb-patches ml, Tristan Gingold, Daniel Jacobowitz

Ping.

Thanks,
Hui

On Fri, Jan 29, 2010 at 17:05, Hui Zhu <teawater@gmail.com> wrote:
> Thanks guys.  I make a new patch according to your mails.
>
> And about the xrealloc, I don't know what I was thinking when I use
> it.  It is really bad, ugly and wasted me a lot of time.  :(
>
> Please help me review the new patch.  Thanks.
>
> Best regards,
> Hui
>
> 2010-01-29  Hui Zhu  <teawater@gmail.com>
>
>        * defs.h (gdb_bfd_errmsg): New extern.
>        * exec.c (exec_file_attach): Change bfd_errmsg to
>        gdb_bfd_errmsg.
>        * utils.c (AMBIGUOUS_MESS1): New macro.
>        (AMBIGUOUS_MESS2): New macro.
>        (gdb_bfd_errmsg): New function.
>
> ---
>  defs.h  |    8 ++++++++
>  exec.c  |    6 ++++--
>  utils.c |   40 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+), 2 deletions(-)
>
> --- a/defs.h
> +++ b/defs.h
> @@ -419,6 +419,13 @@ char **gdb_buildargv (const char *);
>
>  int compare_positive_ints (const void *ap, const void *bp);
>
> +/* A wrapper for bfd_errmsg to produce a more helpful error message
> +   in the case of bfd_error_file_ambiguously recognized.
> +   MATCHING, if non-NULL, is the corresponding argument to
> +   bfd_check_format_matches, and will be freed.  */
> +
> +extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
> +
>  /* From demangle.c */
>
>  extern void set_demangling_style (char *);
> @@ -1226,4 +1233,5 @@ void dummy_obstack_deallocate (void *obj
>  extern void initialize_progspace (void);
>  extern void initialize_inferiors (void);
>
> +
>  #endif /* #ifndef DEFS_H */
> --- a/exec.c
> +++ b/exec.c
> @@ -219,6 +219,7 @@ exec_file_attach (char *filename, int fr
>       char *scratch_pathname;
>       int scratch_chan;
>       struct target_section *sections = NULL, *sections_end = NULL;
> +      char **matching;
>
>       scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, filename,
>                   write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
> @@ -253,13 +254,14 @@ exec_file_attach (char *filename, int fr
>       scratch_pathname = xstrdup (scratch_pathname);
>       cleanups = make_cleanup (xfree, scratch_pathname);
>
> -      if (!bfd_check_format (exec_bfd, bfd_object))
> +      if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
>        {
>          /* Make sure to close exec_bfd, or else "run" might try to use
>             it.  */
>          exec_close ();
>          error (_("\"%s\": not in executable format: %s"),
> -                scratch_pathname, bfd_errmsg (bfd_get_error ()));
> +                scratch_pathname,
> +                gdb_bfd_errmsg (bfd_get_error (), matching));
>        }
>
>       /* FIXME - This should only be run for RS6000, but the ifdef is a poor
> --- a/utils.c
> +++ b/utils.c
> @@ -3608,6 +3608,46 @@ compare_positive_ints (const void *ap, c
>   return * (int *) ap - * (int *) bp;
>  }
>
> +#define AMBIGUOUS_MESS1        ".\nMatching formats:"
> +#define AMBIGUOUS_MESS2        ".\nUse \"set gnutarget format-name\" specify
> the format."
> +
> +const char *
> +gdb_bfd_errmsg (bfd_error_type error_tag, char **matching)
> +{
> +  char *ret, *retp;
> +  int ret_len;
> +  char **p;
> +
> +  /* Check if errmsg just need simple return.  */
> +  if (error_tag != bfd_error_file_ambiguously_recognized || matching == NULL)
> +    return bfd_errmsg (error_tag);
> +
> +  ret_len = strlen (bfd_errmsg (error_tag)) + strlen (AMBIGUOUS_MESS1)
> +            + strlen (AMBIGUOUS_MESS2);
> +  for (p = matching; *p; p++)
> +    ret_len += strlen (*p) + 1;
> +  ret = xmalloc (ret_len + 1);
> +  retp = ret;
> +  make_cleanup (xfree, ret);
> +
> +  strcpy (retp, bfd_errmsg (error_tag));
> +  retp += strlen (retp);
> +
> +  strcpy (retp, AMBIGUOUS_MESS1);
> +  retp += strlen (retp);
> +
> +  for (p = matching; *p; p++)
> +    {
> +      sprintf (retp, " %s", *p);
> +      retp += strlen (retp);
> +    }
> +  xfree (matching);
> +
> +  strcpy (retp, AMBIGUOUS_MESS2);
> +
> +  return ret;
> +}
> +
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
>  extern initialize_file_ftype _initialize_utils;
>


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

* Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
  2010-02-04  7:09                       ` Hui Zhu
@ 2010-02-04 17:38                         ` Doug Evans
  2010-02-05  2:44                           ` Hui Zhu
  0 siblings, 1 reply; 16+ messages in thread
From: Doug Evans @ 2010-02-04 17:38 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Joel Brobecker, Jan Kratochvil, gdb-patches ml, Tristan Gingold,
	Daniel Jacobowitz

On Wed, Feb 3, 2010 at 11:09 PM, Hui Zhu <teawater@gmail.com> wrote:
> Ping.

Righto.  Sorry!

> Thanks,
> Hui
>
> On Fri, Jan 29, 2010 at 17:05, Hui Zhu <teawater@gmail.com> wrote:
>> Thanks guys.  I make a new patch according to your mails.
>>
>> And about the xrealloc, I don't know what I was thinking when I use
>> it.  It is really bad, ugly and wasted me a lot of time.  :(
>>
>> Please help me review the new patch.  Thanks.

The patch is fine with me with two nits noted below.
No need to resubmit, you can just check it in with the requested changes.

>>
>> Best regards,
>> Hui
>>
>> 2010-01-29  Hui Zhu  <teawater@gmail.com>
>>
>>        * defs.h (gdb_bfd_errmsg): New extern.
>>        * exec.c (exec_file_attach): Change bfd_errmsg to
>>        gdb_bfd_errmsg.
>>        * utils.c (AMBIGUOUS_MESS1): New macro.
>>        (AMBIGUOUS_MESS2): New macro.
>>        (gdb_bfd_errmsg): New function.
>>
>> ---
>>  defs.h  |    8 ++++++++
>>  exec.c  |    6 ++++--
>>  utils.c |   40 ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 52 insertions(+), 2 deletions(-)
>>
>> --- a/defs.h
>> +++ b/defs.h
>> @@ -419,6 +419,13 @@ char **gdb_buildargv (const char *);
>>
>>  int compare_positive_ints (const void *ap, const void *bp);
>>
>> +/* A wrapper for bfd_errmsg to produce a more helpful error message
>> +   in the case of bfd_error_file_ambiguously recognized.
>> +   MATCHING, if non-NULL, is the corresponding argument to
>> +   bfd_check_format_matches, and will be freed.  */
>> +
>> +extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
>> +
>>  /* From demangle.c */
>>
>>  extern void set_demangling_style (char *);
>> @@ -1226,4 +1233,5 @@ void dummy_obstack_deallocate (void *obj
>>  extern void initialize_progspace (void);
>>  extern void initialize_inferiors (void);
>>
>> +
>>  #endif /* #ifndef DEFS_H */

(1) Please remove the added blank line.

>> --- a/exec.c
>> +++ b/exec.c
>> @@ -219,6 +219,7 @@ exec_file_attach (char *filename, int fr
>>       char *scratch_pathname;
>>       int scratch_chan;
>>       struct target_section *sections = NULL, *sections_end = NULL;
>> +      char **matching;
>>
>>       scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, filename,
>>                   write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
>> @@ -253,13 +254,14 @@ exec_file_attach (char *filename, int fr
>>       scratch_pathname = xstrdup (scratch_pathname);
>>       cleanups = make_cleanup (xfree, scratch_pathname);
>>
>> -      if (!bfd_check_format (exec_bfd, bfd_object))
>> +      if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
>>        {
>>          /* Make sure to close exec_bfd, or else "run" might try to use
>>             it.  */
>>          exec_close ();
>>          error (_("\"%s\": not in executable format: %s"),
>> -                scratch_pathname, bfd_errmsg (bfd_get_error ()));
>> +                scratch_pathname,
>> +                gdb_bfd_errmsg (bfd_get_error (), matching));
>>        }
>>
>>       /* FIXME - This should only be run for RS6000, but the ifdef is a poor
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -3608,6 +3608,46 @@ compare_positive_ints (const void *ap, c
>>   return * (int *) ap - * (int *) bp;
>>  }
>>
>> +#define AMBIGUOUS_MESS1        ".\nMatching formats:"
>> +#define AMBIGUOUS_MESS2        ".\nUse \"set gnutarget format-name\" specify the format."

(2) Grammar nit.
s,format-name\" specify,format-name\" to specify,

[insert "to" before "specify"]

>> +
>> +const char *
>> +gdb_bfd_errmsg (bfd_error_type error_tag, char **matching)
>> +{
>> +  char *ret, *retp;
>> +  int ret_len;
>> +  char **p;
>> +
>> +  /* Check if errmsg just need simple return.  */
>> +  if (error_tag != bfd_error_file_ambiguously_recognized || matching == NULL)
>> +    return bfd_errmsg (error_tag);
>> +
>> +  ret_len = strlen (bfd_errmsg (error_tag)) + strlen (AMBIGUOUS_MESS1)
>> +            + strlen (AMBIGUOUS_MESS2);
>> +  for (p = matching; *p; p++)
>> +    ret_len += strlen (*p) + 1;
>> +  ret = xmalloc (ret_len + 1);
>> +  retp = ret;
>> +  make_cleanup (xfree, ret);
>> +
>> +  strcpy (retp, bfd_errmsg (error_tag));
>> +  retp += strlen (retp);
>> +
>> +  strcpy (retp, AMBIGUOUS_MESS1);
>> +  retp += strlen (retp);
>> +
>> +  for (p = matching; *p; p++)
>> +    {
>> +      sprintf (retp, " %s", *p);
>> +      retp += strlen (retp);
>> +    }
>> +  xfree (matching);
>> +
>> +  strcpy (retp, AMBIGUOUS_MESS2);
>> +
>> +  return ret;
>> +}
>> +
>>  /* Provide a prototype to silence -Wmissing-prototypes.  */
>>  extern initialize_file_ftype _initialize_utils;
>>
>


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

* Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized
  2010-02-04 17:38                         ` Doug Evans
@ 2010-02-05  2:44                           ` Hui Zhu
  0 siblings, 0 replies; 16+ messages in thread
From: Hui Zhu @ 2010-02-05  2:44 UTC (permalink / raw)
  To: Doug Evans
  Cc: Joel Brobecker, Jan Kratochvil, gdb-patches ml, Tristan Gingold,
	Daniel Jacobowitz

Fixed and checked in.
Thanks Doug.

Best regards,
Hui

On Fri, Feb 5, 2010 at 01:38, Doug Evans <dje@google.com> wrote:
> On Wed, Feb 3, 2010 at 11:09 PM, Hui Zhu <teawater@gmail.com> wrote:
>> Ping.
>
> Righto.  Sorry!
>
>> Thanks,
>> Hui
>>
>> On Fri, Jan 29, 2010 at 17:05, Hui Zhu <teawater@gmail.com> wrote:
>>> Thanks guys.  I make a new patch according to your mails.
>>>
>>> And about the xrealloc, I don't know what I was thinking when I use
>>> it.  It is really bad, ugly and wasted me a lot of time.  :(
>>>
>>> Please help me review the new patch.  Thanks.
>
> The patch is fine with me with two nits noted below.
> No need to resubmit, you can just check it in with the requested changes.
>
>>>
>>> Best regards,
>>> Hui
>>>
>>> 2010-01-29  Hui Zhu  <teawater@gmail.com>
>>>
>>>        * defs.h (gdb_bfd_errmsg): New extern.
>>>        * exec.c (exec_file_attach): Change bfd_errmsg to
>>>        gdb_bfd_errmsg.
>>>        * utils.c (AMBIGUOUS_MESS1): New macro.
>>>        (AMBIGUOUS_MESS2): New macro.
>>>        (gdb_bfd_errmsg): New function.
>>>
>>> ---
>>>  defs.h  |    8 ++++++++
>>>  exec.c  |    6 ++++--
>>>  utils.c |   40 ++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 52 insertions(+), 2 deletions(-)
>>>
>>> --- a/defs.h
>>> +++ b/defs.h
>>> @@ -419,6 +419,13 @@ char **gdb_buildargv (const char *);
>>>
>>>  int compare_positive_ints (const void *ap, const void *bp);
>>>
>>> +/* A wrapper for bfd_errmsg to produce a more helpful error message
>>> +   in the case of bfd_error_file_ambiguously recognized.
>>> +   MATCHING, if non-NULL, is the corresponding argument to
>>> +   bfd_check_format_matches, and will be freed.  */
>>> +
>>> +extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
>>> +
>>>  /* From demangle.c */
>>>
>>>  extern void set_demangling_style (char *);
>>> @@ -1226,4 +1233,5 @@ void dummy_obstack_deallocate (void *obj
>>>  extern void initialize_progspace (void);
>>>  extern void initialize_inferiors (void);
>>>
>>> +
>>>  #endif /* #ifndef DEFS_H */
>
> (1) Please remove the added blank line.
>
>>> --- a/exec.c
>>> +++ b/exec.c
>>> @@ -219,6 +219,7 @@ exec_file_attach (char *filename, int fr
>>>       char *scratch_pathname;
>>>       int scratch_chan;
>>>       struct target_section *sections = NULL, *sections_end = NULL;
>>> +      char **matching;
>>>
>>>       scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, filename,
>>>                   write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
>>> @@ -253,13 +254,14 @@ exec_file_attach (char *filename, int fr
>>>       scratch_pathname = xstrdup (scratch_pathname);
>>>       cleanups = make_cleanup (xfree, scratch_pathname);
>>>
>>> -      if (!bfd_check_format (exec_bfd, bfd_object))
>>> +      if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
>>>        {
>>>          /* Make sure to close exec_bfd, or else "run" might try to use
>>>             it.  */
>>>          exec_close ();
>>>          error (_("\"%s\": not in executable format: %s"),
>>> -                scratch_pathname, bfd_errmsg (bfd_get_error ()));
>>> +                scratch_pathname,
>>> +                gdb_bfd_errmsg (bfd_get_error (), matching));
>>>        }
>>>
>>>       /* FIXME - This should only be run for RS6000, but the ifdef is a poor
>>> --- a/utils.c
>>> +++ b/utils.c
>>> @@ -3608,6 +3608,46 @@ compare_positive_ints (const void *ap, c
>>>   return * (int *) ap - * (int *) bp;
>>>  }
>>>
>>> +#define AMBIGUOUS_MESS1        ".\nMatching formats:"
>>> +#define AMBIGUOUS_MESS2        ".\nUse \"set gnutarget format-name\" specify the format."
>
> (2) Grammar nit.
> s,format-name\" specify,format-name\" to specify,
>
> [insert "to" before "specify"]
>
>>> +
>>> +const char *
>>> +gdb_bfd_errmsg (bfd_error_type error_tag, char **matching)
>>> +{
>>> +  char *ret, *retp;
>>> +  int ret_len;
>>> +  char **p;
>>> +
>>> +  /* Check if errmsg just need simple return.  */
>>> +  if (error_tag != bfd_error_file_ambiguously_recognized || matching == NULL)
>>> +    return bfd_errmsg (error_tag);
>>> +
>>> +  ret_len = strlen (bfd_errmsg (error_tag)) + strlen (AMBIGUOUS_MESS1)
>>> +            + strlen (AMBIGUOUS_MESS2);
>>> +  for (p = matching; *p; p++)
>>> +    ret_len += strlen (*p) + 1;
>>> +  ret = xmalloc (ret_len + 1);
>>> +  retp = ret;
>>> +  make_cleanup (xfree, ret);
>>> +
>>> +  strcpy (retp, bfd_errmsg (error_tag));
>>> +  retp += strlen (retp);
>>> +
>>> +  strcpy (retp, AMBIGUOUS_MESS1);
>>> +  retp += strlen (retp);
>>> +
>>> +  for (p = matching; *p; p++)
>>> +    {
>>> +      sprintf (retp, " %s", *p);
>>> +      retp += strlen (retp);
>>> +    }
>>> +  xfree (matching);
>>> +
>>> +  strcpy (retp, AMBIGUOUS_MESS2);
>>> +
>>> +  return ret;
>>> +}
>>> +
>>>  /* Provide a prototype to silence -Wmissing-prototypes.  */
>>>  extern initialize_file_ftype _initialize_utils;
>>>
>>
>


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

end of thread, other threads:[~2010-02-05  2:44 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-14  2:44 [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized Hui Zhu
2010-01-14 10:15 ` Jan Kratochvil
2010-01-19  7:01   ` Hui Zhu
2010-01-19  7:57     ` Jan Kratochvil
2010-01-19  8:08       ` Hui Zhu
2010-01-26  8:01         ` Hui Zhu
2010-01-26 21:04           ` Doug Evans
2010-01-27  9:01             ` Hui Zhu
2010-01-28 18:18               ` Doug Evans
2010-01-28 21:56               ` Jan Kratochvil
2010-01-28 21:59                 ` Doug Evans
2010-01-29  3:51                   ` Joel Brobecker
2010-01-29  9:06                     ` Hui Zhu
2010-02-04  7:09                       ` Hui Zhu
2010-02-04 17:38                         ` Doug Evans
2010-02-05  2:44                           ` Hui Zhu

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