RFR: 8185757: QuickSort array size should be size_t

Kim Barrett kim.barrett at oracle.com
Tue Aug 8 00:27:16 UTC 2017


> On Aug 7, 2017, at 5:42 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi,
> 
> On Fri, 2017-08-04 at 19:20 -0400, Kim Barrett 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) {
>> 
> 
>   looks good.
> 
> Thanks a lot.
> 
> Thomas

Thanks.



More information about the hotspot-runtime-dev mailing list