Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tim Auton <tim.auton@uton.org>
To: gdb-patches@sourceware.org
Subject: Remote Debugging Protocol - hex case
Date: Tue, 13 Feb 2007 21:45:00 -0000	[thread overview]
Message-ID: <842A2789-AFD7-4339-8E5E-608D6C9CFDD7@uton.org> (raw)

I just had a problem when using a remote debugging stub with GDB 6.6.
The stub is part of Avrora, a simulator for the Atmel AVR
microcontroller. The problem is that Avrora was sending replies using
upper-case hex, so when a hex reply started with "E" it was *sometimes*
interpreted as an error. Obviously, fixing the stub to use lower case
for hex is on the agenda. But in looking through the GDB documentation
and remote.c source file I found a number of inconsistencies which are
probably worth addressing.

First, I could find nowhere in the documentation that specified stubs
must use lower-case for hex. If they should (and of course they
should, because upper-case doesn't work properly), it should be in the
docs.

Even looking through the relevant source (remote.c), at first glance it
looks like it *does* support upper-case hex. The fromhex() function
supports upper case and there are a couple of functions -
packet_check_result() and packet_ok() - which implement checking which
can distinguish upper-case hex replies from true error messages. But
those functions simply aren't used, other than in remote_send_printf().
The functions remote_read_bytes() and remote_rcmd() each implement
something like packet_check_result() and will correctly handle
upper-case hex as a result. All the other functions treat any reply
which starts with an 'E' as an error, but will accept all other upper
case hex replies. The upshot of all this is that it's pretty easy to
write a stub which uses upper-case hex and for it to appear to work just
fine.

As far as I can see, making remote debugging support upper-case hex
properly won't be much trouble. It pretty much amounts to replacing the
disparate error checking in remote.c with calls to
packet_check_result(). But it's quite possible that I'm missing wider
repercussions which are obvious to someone who knows GDB and the source
much better than me (which is probably most of you). In particular, if
there are any stubs in the wild which reply with something other than
the documented error codes Exx (where x is a hex digit) or E.something
and expect it to be treated as an error, supporting upper-case hex might
break them. On the other hand, looking at the mailing list archives, not
supporting upper case hex consistently has caused some breakage too.

In summary, I see three options:

1) Patch remote.c to support upper-case hex consistently, update docs to
suggest lower-case for backward compatibility.

	Pros: Consistent with the rest of GDB, which generally supports
	upper-case hex.

	Cons: Any stubs which don't provide a two-digit errno or a E.something
	string will break. (Do any exist?)

2) Update docs to require lower-case, remove partial support for
upper-case hex from remote.c to avoid confusion.

	Pros: Should only break stubs which are intermittently broken so
	need fixing anyway; avoids misleading testing results when
	developing new stubs.

	Cons: Inconsistent with rest of GDB, which generally supports
	upper-case hex. Will cause some breakage, if only of stubs which are
	intermittently broken anyway.

3) Do nothing.

	Pros: Zero effort.

	Cons: People writing new stubs will continue to get bitten by the
	inconsistency.

I'm happy to write and submit the patches once the maintainers decide on
the best course of action, though as I'm not intimate with GDB's
internals and don't (yet) have the first clue about texinfo (but am
wiling to learn), the maintainers may wish to be more circumspect than
ususal in checking them. I've found the GNU coding standards, but
pointers to anything else relevant would be well received (off-list, if
appropriate).

Thoughts?


Tim


             reply	other threads:[~2007-02-13 21:45 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-13 21:45 Tim Auton [this message]
2007-02-13 22:49 ` Daniel Jacobowitz
2007-02-14 16:26   ` Tim Auton
2007-02-14 16:41     ` 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=842A2789-AFD7-4339-8E5E-608D6C9CFDD7@uton.org \
    --to=tim.auton@uton.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