RFR: 8185757: QuickSort array size should be size_t

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Tue Aug 8 00:22:42 UTC 2017



This looks good.  I don't think we'll change the methods->length() 
because it's limited to u2 and used to set method_idnum() which is a 
u2.  Odd that the compiler doesn't complain about narrowing an int into 
a u2 though.

Coleen

On 8/4/17 7:20 PM, Kim Barrett wrote:
>> On Aug 3, 2017, at 3:05 PM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>
>> Hi,
>>
>> On Thu, 2017-08-03 at 14:06 -0400, Kim Barrett wrote:
>>>> On Aug 3, 2017, at 8:32 AM, Thomas Schatzl <thomas.schatzl at oracle.c
>>>> om> wrote:
>>>> However, please put the closing brackets of these into extra lines
>>>> (quicksort.hpp:76,77) to avoid the casual reader to overlook them.
>>> Sorry, but that just looks horrible.  As a casual reader, I wouldn’t
>>> even look for them, since if they aren’t there then the code is
>>> badly mis-indented.
>> Actually I was already at writing about an issue with indentation when
>> I noticed the brackets :)
>>
>> Not insisting on changing this.
> I added a couple of asserts to check for buffer overruns due to a bad comparator:
>
> diff -r 6b62ed03a6a6 -r 7616ceb92653 src/share/vm/utilities/quickSort.hpp
> --- a/src/share/vm/utilities/quickSort.hpp	Fri Aug 04 18:02:51 2017 -0400
> +++ b/src/share/vm/utilities/quickSort.hpp	Fri Aug 04 19:17:36 2017 -0400
> @@ -73,8 +73,12 @@
>       T pivot_val = array[pivot];
>   
>       for ( ; true; ++left_index, --right_index) {
> -      for ( ; comparator(array[left_index], pivot_val) < 0; ++left_index) {}
> -      for ( ; comparator(array[right_index], pivot_val) > 0; --right_index) {}
> +      for ( ; comparator(array[left_index], pivot_val) < 0; ++left_index) {
> +        assert(left_index < length, "reached end of partition");
> +      }
> +      for ( ; comparator(array[right_index], pivot_val) > 0; --right_index) {
> +        assert(right_index > 0, "reached start of partition");
> +      }
>   
>         if (left_index < right_index) {
>           if (!idempotent || comparator(array[left_index], array[right_index]) != 0) {
>
>



More information about the hotspot-runtime-dev mailing list