Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Jan Kratochvil <jan.kratochvil@redhat.com>
To: Hui Zhu <teawater@gmail.com>
Cc: gdb-patches ml <gdb-patches@sourceware.org>,
	        Tristan Gingold <gingold@adacore.com>,
	        Daniel Jacobowitz <dan@codesourcery.com>
Subject: Re: [RFA] Show some tips when file cmd get  bfd_error_file_ambiguously_recognized
Date: Thu, 14 Jan 2010 10:15:00 -0000	[thread overview]
Message-ID: <20100114101348.GA24756@host0.dyn.jankratochvil.net> (raw)
In-Reply-To: <daef60381001131843u683d35d2nf937db926cea9740@mail.gmail.com>

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


  reply	other threads:[~2010-01-14 10:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-14  2:44 Hui Zhu
2010-01-14 10:15 ` Jan Kratochvil [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100114101348.GA24756@host0.dyn.jankratochvil.net \
    --to=jan.kratochvil@redhat.com \
    --cc=dan@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    --cc=gingold@adacore.com \
    --cc=teawater@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox