Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Daniel Jacobowitz <drow@false.org>
Cc: gdb-patches@sourceware.org
Subject: Re: [rfc] Let the remote protocol report the target's arch
Date: Fri, 05 Jan 2007 12:56:00 -0000	[thread overview]
Message-ID: <u64bl1vjp.fsf@gnu.org> (raw)
In-Reply-To: <20070104221622.GM24634@nevyn.them.org> (message from Daniel 	Jacobowitz on Thu, 4 Jan 2007 17:16:22 -0500)

> Date: Thu, 4 Jan 2007 17:16:22 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> When you get a chance, could you look at this?  It's not too urgent; I
> need to work on the next patch in this series, since I've forgotten
> where I was over the last month.
> 
> Original patch is here:
> http://sourceware.org/ml/gdb-patches/2006-12/msg00100.html

Sorry, I've managed to miss the fact that portions of this patch await
my review.  On to it now.

> +Access the target's @dfn{target description}.  @xref{Target Descriptions}.  The

``target's target description''?  Wouldn't this be better:

  Access the @dfn{target description}.

> +annex specifies which XML document to access.  The main description is
> +always loaded from @samp{target.xml}.

Is `target.xml' a literal file name, or is the `target' part something
that will be replaced with a specific target name?  If the former,
please use @file{target.xml}, if the latter, @file{@var{target}.xml}.
If it's not a file at all, I think this text needs to explain what it
is.

Similar problems with the markup of target.xml are elsewhere in the
patch.

> +@node Target Descriptions
> +@appendix Target Descriptions

Please add a suitable @cindex entry here, for those who will look for
this stuff because they remember it's described somewhere in the
manual.

> +Alternatively, you can specify a file to read for the target description.
> +If a file is set, the target will not be queried.
> +
> +@table @code
> +@cindex set tdesc filename
> +@item set tdesc filename @var{path}

I suggest a sentence before @table to explain what is described in
this table.

>                                           If @var{path} is empty,
> +use the target-supplied description.

This sentence is unclear, at least to me: what is this target-supplied
description, and where does one find it?  If it's target.xml, then how
about mentioning it here explicitly?

> +@node Target Description Format
> +@section Target Description Format

Suggest an @cindex entry here as well: someone could be looking for
this format description.

> +@example
> +<target>
> +  <architecture>i386:x86-64</architecture>
> +</target>
> +@end example
> +
> +@noindent
> +This description only says that the target uses the x86-64 architecture.
> +describes a simple target feature set which only contains two
> +registers, named @code{s0} (a 32-bit integer register) and @code{s1}
> +(a 32-bit floating point register).

Something's missing here, before the "describes a simple target" part.

> +The content of the @samp{<architecture>} element is an architecture
> +name, from the same selection accepted by @code{set architecture}

There's no `<architecture>' element in the example that precedes this
text, which is confusing.  You should probably rewrite the example.

> +set tdesc filename
> +show tdesc filename
> +  Read an XML target description from the specified local file instead
> +  of querying the target for its description.

Actually, "set tdesc filename" command doesn't read anything, it just
sets the name of the file from which to read the description.

Anyway, I think NEWS should say explicitly that GDB can now read a
description of remote target's features from an XML file.  Then these
two new commands will make sense to the reader.


  reply	other threads:[~2007-01-05 12:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-07 16:52 Daniel Jacobowitz
2007-01-04 22:16 ` Daniel Jacobowitz
2007-01-05 12:56   ` Eli Zaretskii [this message]
2007-01-05 19:15     ` Daniel Jacobowitz
2007-01-06 10:29       ` Eli Zaretskii
2007-01-09 19:50         ` Daniel Jacobowitz
2007-01-09 22:17           ` Eli Zaretskii
2007-01-09 22:56             ` Daniel Jacobowitz

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=u64bl1vjp.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=drow@false.org \
    --cc=gdb-patches@sourceware.org \
    /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