Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@redhat.com>
To: Joel Brobecker <brobecker@adacore.com>
Cc: Jie Zhang <jie.zhang@analog.com>, gdb-patches@sourceware.org
Subject: Re: PING: [PATCH] Fix a bug of addrmap
Date: Thu, 27 Nov 2008 14:45:00 -0000	[thread overview]
Message-ID: <m38wr6pgwh.fsf@fleche.redhat.com> (raw)
In-Reply-To: <20081125174613.GF3946@adacore.com> (Joel Brobecker's message of "Tue\, 25 Nov 2008 09\:46\:13 -0800")

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Did you familiarize yourself with the code that you'd say that
Joel> the patch sounds good to you?

I looked at it again today.

I re-read the contract for this function, and it seems pretty clear
that the current behavior is intended:

/* In the mutable address map MAP, associate the addresses from START
   to END_INCLUSIVE that are currently associated with NULL with OBJ
   instead.  Addresses mapped to an object other than NULL are left
   unchanged.

In particular, "currently associated with NULL" is the key.

My understanding now is that this data structure is designed to be
built bottom-up.  So, I think that this is a bug in the caller, and
that the test I wrote is actually not correct.

Of course, it would be ok to change addrmap's contract.  But, if we
want to do this then I think we would need more tests covering other
corner cases.

BTW, addrmap is nice code.  It has all the hallmarks of the Blandy
style.

Tom


  parent reply	other threads:[~2008-11-26 16:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-21  4:44 Jie Zhang
2008-11-03  6:09 ` PING: " Jie Zhang
2008-11-25 23:09   ` Tom Tromey
2008-11-26  6:34     ` Joel Brobecker
2008-11-26 13:53       ` Tom Tromey
2008-11-26 14:24       ` Daniel Jacobowitz
2008-11-26 15:05         ` Daniel Jacobowitz
2008-11-27 14:45       ` Tom Tromey [this message]
2008-12-08 23:12         ` Joel Brobecker

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=m38wr6pgwh.fsf@fleche.redhat.com \
    --to=tromey@redhat.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jie.zhang@analog.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