Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized

[Hui Zhu <teawater at gmail dot com>]

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;