Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-09 Thread Brian Burkhalter
On Oct 9, 2013, at 3:41 AM, Alan Bateman wrote: > On 08/10/2013 21:50, Brian Burkhalter wrote: >> >> I have updated the webrev accordingly >> >> http://cr.openjdk.java.net/~bpb/8016252/webrev.3/ > This looks good, I think this gets us to where we wanted to be. Pushed: http://hg.openjdk.java.net

Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-09 Thread Alan Bateman
On 08/10/2013 21:50, Brian Burkhalter wrote: I have updated the webrev accordingly http://cr.openjdk.java.net/~bpb/8016252/webrev.3/ This looks good, I think this gets us to where we wanted to be. : I skimmed over the test but it doesn't

Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-09 Thread Chris Hegarty
On 08/10/2013 21:50, Brian Burkhalter wrote: http://cr.openjdk.java.net/~bpb/8016252/webrev.3/ This looks good to me. If you haven't already got a sponsor, then I would be happy to sponsor this for you. -Chris.

Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-08 Thread Brian Burkhalter
On Oct 8, 2013, at 1:51 AM, Chris Hegarty wrote: > This looks much better. > > loadfactor and size validation look good, and in line with the original > suggestion in the bug. > > For the initial capacity what was originally suggested was to use a similar > technique to that of HashMap.readObj

Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-08 Thread Alan Bateman
On 07/10/2013 22:03, Brian Burkhalter wrote: On Oct 7, 2013, at 1:43 PM, Brian Burkhalter wrote: On second thought an exception really should be thrown on negative size; will update. http://cr.openjdk.java.net/~bpb/8016252.2/ updated including a not-very-exciting and perhaps unnecessary test

Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-08 Thread Chris Hegarty
On 10/07/2013 10:03 PM, Brian Burkhalter wrote: On Oct 7, 2013, at 1:43 PM, Brian Burkhalter wrote: On second thought an exception really should be thrown on negative size; will update. http://cr.openjdk.java.net/~bpb/8016252.2/ updated including a not-very-exciting and perhaps unnecessary

Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-07 Thread Brian Burkhalter
On Oct 7, 2013, at 1:43 PM, Brian Burkhalter wrote: > On second thought an exception really should be thrown on negative size; will > update. http://cr.openjdk.java.net/~bpb/8016252.2/ updated including a not-very-exciting and perhaps unnecessary test. Brian

Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-07 Thread Brian Burkhalter
On Oct 7, 2013, at 1:31 PM, Alan Bateman wrote: > For size then I don't think the Math.max has any real effect because the loop > don't do anything if size is negative You could just throw > IllegalObjectException if it is negative (as per the first patch) if you > really wanted to. On second

Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-07 Thread Brian Burkhalter
On Oct 7, 2013, at 1:31 PM, Alan Bateman wrote: > I'm not so sure about capacity, a simple check if it is negative should be > sufficient. The idea was to try to use the value if it appears reasonable, which is assured by the clamping. > For size then I don't think the Math.max has any real e

Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-07 Thread Alan Bateman
On 07/10/2013 18:40, Brian Burkhalter wrote: : I have posted an updated webrev here: http://cr.openjdk.java.net/~bpb/8016252.2/ I don't know whether it is the correct way to go, but this version attempts to use the capacity, load factor, and size as read in, insofar as they appear reasonable.

Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-07 Thread Brian Burkhalter
On Oct 3, 2013, at 10:14 PM, Alan Bateman wrote: > On 03/10/2013 16:17, Brian Burkhalter wrote: >> Please review and comment at your convenience. >> >> Issue: https://bugs.openjdk.java.net/browse/JDK-8016252 >> Webrev: http://cr.openjdk.java.net/~bpb/8016252/ >> >> Summary >> * Improv

Re: JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-03 Thread Alan Bateman
On 03/10/2013 16:17, Brian Burkhalter wrote: Please review and comment at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8016252 Webrev: http://cr.openjdk.java.net/~bpb/8016252/ Summary * Improve validation checks in HashSet.readObject(). Would this change imply updating th

JDK 8 RFR 8016252: More defensive HashSet.readObject

2013-10-03 Thread Brian Burkhalter
Please review and comment at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8016252 Webrev: http://cr.openjdk.java.net/~bpb/8016252/ Summary * Improve validation checks in HashSet.readObject(). Would this change imply updating the serialVersionUID? Thanks, Brian