* [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