From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5145 invoked by alias); 28 Jan 2010 18:18:16 -0000 Received: (qmail 5134 invoked by uid 22791); 28 Jan 2010 18:18:14 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,SARE_MSGID_LONG40,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.33.17) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 28 Jan 2010 18:18:10 +0000 Received: from wpaz13.hot.corp.google.com (wpaz13.hot.corp.google.com [172.24.198.77]) by smtp-out.google.com with ESMTP id o0SII5lK008413 for ; Thu, 28 Jan 2010 18:18:06 GMT Received: from fxm28 (fxm28.prod.google.com [10.184.13.28]) by wpaz13.hot.corp.google.com with ESMTP id o0SII3fw015517 for ; Thu, 28 Jan 2010 10:18:04 -0800 Received: by fxm28 with SMTP id 28so966743fxm.0 for ; Thu, 28 Jan 2010 10:18:03 -0800 (PST) MIME-Version: 1.0 Received: by 10.216.89.213 with SMTP id c63mr1103583wef.211.1264702683203; Thu, 28 Jan 2010 10:18:03 -0800 (PST) In-Reply-To: References: <20100114101348.GA24756@host0.dyn.jankratochvil.net> <20100119075642.GA30154@host0.dyn.jankratochvil.net> Date: Thu, 28 Jan 2010 18:18:00 -0000 Message-ID: Subject: Re: [RFA] Show some tips when file cmd get bfd_error_file_ambiguously_recognized From: Doug Evans To: Hui Zhu Cc: Jan Kratochvil , gdb-patches ml , Tristan Gingold , Daniel Jacobowitz Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-01/txt/msg00609.txt.bz2 On Wed, Jan 27, 2010 at 1:00 AM, Hui Zhu wrote: > Agree with you. =A0I 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 =A0Hui Zhu =A0 > > =A0 =A0 =A0 =A0* defs.h (gdb_bfd_errmsg): New extern. > =A0 =A0 =A0 =A0* exec.c (exec_file_attach): Change bfd_errmsg to > =A0 =A0 =A0 =A0gdb_bfd_errmsg. > =A0 =A0 =A0 =A0* utils.c (AMBIGUOUS_MESS1): New macro. > =A0 =A0 =A0 =A0(AMBIGUOUS_MESS2): New macro. > =A0 =A0 =A0 =A0(gdb_bfd_errmsg): New function. > > --- > =A0defs.h =A0| =A0 =A08 ++++++++ > =A0exec.c =A0| =A0 =A06 ++++-- > =A0utils.c | =A0 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > =A03 files changed, 64 insertions(+), 2 deletions(-) > > --- a/defs.h > +++ b/defs.h > @@ -1226,4 +1226,12 @@ void dummy_obstack_deallocate (void *obj > =A0extern void initialize_progspace (void); > =A0extern void initialize_inferiors (void); > > +/* The wrapper for the bfd_errmsg. > + =A0 When error_tag is bfd_error_file_ambiguously_recognized and matching > + =A0 is not NULL, this function will return more help message to handle > + =A0 this issue. =A0*/ 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 **matc= hing); > + > + > =A0#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 > =A0 =A0 =A0 char *scratch_pathname; > =A0 =A0 =A0 int scratch_chan; > =A0 =A0 =A0 struct target_section *sections =3D NULL, *sections_end =3D N= ULL; > + =A0 =A0 =A0char **matching; > > =A0 =A0 =A0 scratch_chan =3D openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, f= ilename, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 write_files ? O_RDWR | O_BINARY : O_R= DONLY | O_BINARY, > @@ -253,13 +254,14 @@ exec_file_attach (char *filename, int fr > =A0 =A0 =A0 scratch_pathname =3D xstrdup (scratch_pathname); > =A0 =A0 =A0 cleanups =3D make_cleanup (xfree, scratch_pathname); > > - =A0 =A0 =A0if (!bfd_check_format (exec_bfd, bfd_object)) > + =A0 =A0 =A0if (!bfd_check_format_matches (exec_bfd, bfd_object, &matchi= ng)) > =A0 =A0 =A0 =A0{ > =A0 =A0 =A0 =A0 =A0/* Make sure to close exec_bfd, or else "run" might tr= y to use > =A0 =A0 =A0 =A0 =A0 =A0 it. =A0*/ > =A0 =A0 =A0 =A0 =A0exec_close (); > =A0 =A0 =A0 =A0 =A0error (_("\"%s\": not in executable format: %s"), > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0scratch_pathname, bfd_errmsg (bfd_get_er= ror ())); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0scratch_pathname, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0gdb_bfd_errmsg (bfd_get_error (), matchi= ng)); > =A0 =A0 =A0 =A0} > > =A0 =A0 =A0 /* 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 > =A0 return * (int *) ap - * (int *) bp; > =A0} > > +#define AMBIGUOUS_MESS1 =A0 =A0 =A0 =A0", it matches formats:" > +#define AMBIGUOUS_MESS2 =A0 =A0 =A0 =A0", \"set gnutarget format-name\" = handle it" > + > +const char * > +gdb_bfd_errmsg (bfd_error_type error_tag, char **matching) > +{ > + =A0char *ret; > + =A0int ret_len, current_ret_len =3D 0; > + =A0char **p; > + > + =A0/* Check if errmsg just need simple return. =A0*/ > + =A0if (error_tag !=3D bfd_error_file_ambiguously_recognized || !matchin= g) > + =A0 =A0return bfd_errmsg (error_tag); I believe the convention is to write matching =3D=3D NULL instead of !match= ing. > + > + =A0ret_len =3D strlen (bfd_errmsg (error_tag)) + strlen (AMBIGUOUS_MESS= 1) > + =A0 =A0 =A0 =A0 =A0 + 50 + strlen (AMBIGUOUS_MESS2); > + =A0ret =3D 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. > + > + =A0/* bfd_errmsg (error_tag) */ There's no real need for this comment, it doesn't contribute enough (IMO anyway). Ditto for similar comments below. > + =A0strcpy (ret + current_ret_len, bfd_errmsg (error_tag)); > + =A0current_ret_len +=3D strlen (bfd_errmsg (error_tag)); I'd write strcpy (p, ...); p +=3D 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.] > + > + =A0/* AMBIGUOUS_MESS1 */ > + =A0strcpy (ret + current_ret_len, AMBIGUOUS_MESS1); > + =A0current_ret_len +=3D strlen (AMBIGUOUS_MESS1); > + > + =A0/* matching */ > + =A0for (p =3D matching; *p; p++) > + =A0 =A0{ > + =A0 =A0 =A0if (current_ret_len + strlen (*p) + 1 > ret_len) > + =A0 =A0 =A0 =A0{ > + =A0 =A0 =A0 =A0 =A0ret_len +=3D 50; > + =A0 =A0 =A0 =A0 =A0ret =3D xrealloc (ret, ret_len + 1); > + =A0 =A0 =A0 =A0} > + =A0 =A0 =A0sprintf (ret + current_ret_len, " %s", *p); > + =A0 =A0 =A0current_ret_len +=3D strlen (*p) + 1; > + =A0 =A0} > + =A0xfree (matching); > + > + =A0/* AMBIGUOUS_MESS2 */ > + =A0if (current_ret_len + strlen (AMBIGUOUS_MESS2) > ret_len) > + =A0 =A0{ > + =A0 =A0 =A0ret_len +=3D strlen (AMBIGUOUS_MESS2); > + =A0 =A0 =A0ret =3D xrealloc (ret, ret_len + 1); > + =A0 =A0} > + =A0strcpy (ret + current_ret_len, AMBIGUOUS_MESS2); > + > + =A0make_cleanup (xfree, ret); > + > + =A0return ret; > +} > + > =A0/* Provide a prototype to silence -Wmissing-prototypes. =A0*/ > =A0extern initialize_file_ftype _initialize_utils; >