From: Tom Tromey <tromey@redhat.com>
To: Hilfinger@adacore.com
Cc: gdb-patches@sourceware.org, brobecker@adacore.com
Subject: Re: [RFA] 64-bit range types in GDB
Date: Fri, 11 Dec 2009 19:06:00 -0000 [thread overview]
Message-ID: <m3pr6lz5yb.fsf@fleche.redhat.com> (raw)
In-Reply-To: <20091211100306.4A2A9227B32@nile.gnat.com> (Paul Hilfinger's message of "Fri, 11 Dec 2009 05:03:06 -0500 (EST)")
>>>>> "Paul" == Paul Hilfinger <Hilfinger@adacore.com> writes:
Paul> Here is the 64-bit range patch, revised per Tom's suggestions.
Paul> For now, I have eliminated the fixup_range_type_hack routine until
Paul> such time if ever that I find the failing cases that required
Paul> it. (Besides which, I see that the first patch didn't (ahem)
Paul> bother to call it anyway).
:-)
Paul> In check_typedef, I have handled the overflow situation by setting the
Paul> array length to 0. I decided against using UINT_MAX given the likely
Paul> consequences of having such a value actually used to allocate a value
Paul> structure.
Thanks.
Paul> OK to commit?
One nit...
Paul> + /* For now, we conservatively take the array length to be 0
Paul> + * if its length exceeds UINT_MAX. The code below assumes
Paul> + * that for x < 0, (ULONGEST) x == -x + ULONGEST_MAX + 1,
Paul> + * which is technically not guaranteed by C, but is usually true
Paul> + * (because it would be true if x were unsigned with its
Paul> + * high-order bit on). It uses the fact that
Paul> + * high_bound-low_bound is always representable in
Paul> + * ULONGEST and that if high_bound-low_bound+1 overflows,
Paul> + * it overflows to 0. We must change these tests if we
Paul> + * decide to increase the representation of TYPE_LENGTH
Paul> + * from unsigned int to ULONGEST. FIXME: pnh/2009-12-07. */
The GNU style is not to put a "*" at the start of each line. Also we
currently eschew new FIXME comments. I think you could just remove the
FIXME. I personally also am not in favor of putting one's initials in
comments -- gdb has several such comments where I have no clue at all
who the person might be.
Ok with that change.
Tom
next prev parent reply other threads:[~2009-12-11 19:06 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-04 8:13 Paul Hilfinger
2009-12-04 18:35 ` Tom Tromey
2009-12-05 12:18 ` Paul Hilfinger
2009-12-05 16:53 ` Pierre Muller
2009-12-07 20:08 ` Tom Tromey
2009-12-11 10:03 ` Paul Hilfinger
2009-12-11 19:06 ` Tom Tromey [this message]
2009-12-14 6:24 ` [COMMIT] " Paul Hilfinger
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=m3pr6lz5yb.fsf@fleche.redhat.com \
--to=tromey@redhat.com \
--cc=Hilfinger@adacore.com \
--cc=brobecker@adacore.com \
--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