RFR: 8185757: QuickSort array size should be size_t

Thomas Schatzl thomas.schatzl at oracle.com
Mon Aug 7 09:42:34 UTC 2017


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



More information about the hotspot-runtime-dev mailing list