Hi Steve,
I was one of the people that provided feedback on Mike's patch. In my case, it
was a mishap of reply to sender vs. reply to all. I don't have the original
email but, the result are visible in the test case that Mike wrote. My main
concern with the old patch that if you use a raw type (think legacy code) you
can poison a TreeMap/TreeSet with non-null object that cannot be compared. I
remembered reviewing that code back in JSR166 maintenance review. I can't take
credit for it since the review section of the bug report does a great job of
explaining the evolution of the correct patch.
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5045147
Jason
> Subject: Re: Review Request -- 5045147 : When TreeMap is empty explicitly
> check for null keys in put() [updated]
> From: [email protected]
> Date: Tue, 15 Mar 2011 09:34:00 -0700
> To: [email protected]
> CC: [email protected]
>
>
> On Mar 15 2011, at 02:36 , Steve Poole wrote:
>
> > On 14/03/11 21:02, Mike Duigou wrote:
> >> I've gotten feedback regarding this issue and I've updated the webrev to
> >> use the commented out compare(key, key) test rather than the previously
> >> committed solution. I hadn't looked at the commented out code too
> >> carefully and had assumed it was pseudo-code rather than an actual
> >> solution. It's an improvement over the original solution and reads, to me
> >> and apparently others, a lot simpler.
> >>
> > Hi - can you post the feedback to the mailing list?
>
> Hi Steve;
>
> Sorry, I can't repost them. I'd have preferred that the feedback went to the
> list but for whatever reasons the two respondents chose to send the feedback
> privately and I won't repost their private emails. (They are certainly
> welcome to do so if they feel it matters). Perhaps it was just a case of
> "Reply to Sender" vs "Reply to All". I don't know. To summarize the messages
> though, "Just use the commented out code for 504517. It's better than the old
> patch."
>
> Mike
>
> >> http://cr.openjdk.java.net/~mduigou/5045147/1/webrev/
> >>
> >> Also now included is a jtreg unit test.
> >>
> >> Mike
> >
>
