Skip to content

Commit b9f7e03

Browse files
Exit early for trivial sort cases to avoid segfault (#229)
* initial fix * more tests * NEWS
1 parent b4f4a54 commit b9f7e03

3 files changed

Lines changed: 22 additions & 6 deletions

File tree

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
1. `orderfin(as.integer64(10:1), 1:3, 8:11)` enforces that `table` be sorted by `order` instead of segfaulting (#166).
6565
1. `ordertab()` no longer segfaults when `nunique` is smaller than the actual number of unique values (#168).
6666
1. `as.integer64.character` now returns `NA` for out of range values, with warning, e.g. `as.integer64("22222222222222222222")`. Thanks @hcirellu.
67+
1. `quicksort()` and others no longer segfault on trivial cases (e.g. sorting 0 or 1 item, #220).
6768

6869
## NOTES
6970

src/sort64.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ void ram_integer64_shellsort_asc(ValueT *data, IndexT l, IndexT r)
379379
{
380380
ValueT v;
381381
IndexT i, j, h, lh, t, n=r-l+1;
382+
if (n < 2) return;
382383
for (t = 0; shellincs[t] > n; t++);
383384
for (h = shellincs[t]; t < SHELLARRAYSIZE; h = shellincs[++t]){
384385
lh = l+h;
@@ -397,6 +398,7 @@ void ram_integer64_shellsort_desc(ValueT *data, IndexT l, IndexT r)
397398
{
398399
ValueT v;
399400
IndexT i, j, h, lh, t, n=r-l+1;
401+
if (n < 2) return;
400402
for (t = 0; shellincs[t] > n; t++);
401403
for (h = shellincs[t]; t < SHELLARRAYSIZE; h = shellincs[++t]){
402404
lh = l+h;
@@ -416,6 +418,7 @@ void ram_integer64_shellsortorder_asc(ValueT *data, IndexT *index, IndexT l, Ind
416418
{
417419
ValueT v;
418420
IndexT vi, i, j, h, lh, t, n=r-l+1;
421+
if (n < 2) return;
419422
for (t = 0; shellincs[t] > n; t++);
420423
for (h = shellincs[t]; t < SHELLARRAYSIZE; h = shellincs[++t]){
421424
lh = l+h;
@@ -437,6 +440,7 @@ void ram_integer64_shellsortorder_desc(ValueT *data, IndexT *index, IndexT l, In
437440
{
438441
ValueT v;
439442
IndexT vi, i, j, h, lh, t, n=r-l+1;
443+
if (n < 2) return;
440444
for (t = 0; shellincs[t] > n; t++);
441445
for (h = shellincs[t]; t < SHELLARRAYSIZE; h = shellincs[++t]){
442446
lh = l+h;
@@ -459,6 +463,7 @@ void ram_integer64_shellorder_asc(ValueT *data, IndexT *index, IndexT l, IndexT
459463
{
460464
ValueT v;
461465
IndexT vi, i, j, h, lh, t, n=r-l+1;
466+
if (n < 2) return;
462467
for (t = 0; shellincs[t] > n; t++);
463468
for (h = shellincs[t]; t < SHELLARRAYSIZE; h = shellincs[++t]){
464469
lh = l+h;
@@ -478,6 +483,7 @@ void ram_integer64_shellorder_desc(ValueT *data, IndexT *index, IndexT l, IndexT
478483
{
479484
ValueT v;
480485
IndexT vi, i, j, h, lh, t, n=r-l+1;
486+
if (n < 2) return;
481487
for (t = 0; shellincs[t] > n; t++);
482488
for (h = shellincs[t]; t < SHELLARRAYSIZE; h = shellincs[++t]){
483489
lh = l+h;

tests/testthat/test-sort64.R

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,13 +254,22 @@ test_that("Shellsort direct invocation", {
254254
test_that("Corner cases for partitioning logic", {
255255
# Single element and empty vectors often trip up "do { ... } while" or sentinel loops
256256

257-
# TODO(#220): restore this.
258257
# Case 1: Empty
259-
# x_empty = integer64()
260-
# x = bit::clone(x_empty)
261-
# # bit::quicksort returns the NA count (0L), and modifies 'x' in-place
262-
# expect_identical(bit::quicksort(x), 0L)
263-
# expect_identical(x, x_empty)
258+
x_empty = integer64()
259+
x = bit::clone(x_empty)
260+
# bit::quicksort returns the NA count (0L), and modifies 'x' in-place
261+
expect_identical(bit::quicksort(x), 0L)
262+
expect_identical(x, x_empty)
263+
expect_identical(bit::quicksort(x, decreasing = TRUE), 0L)
264+
expect_identical(x, x_empty)
265+
266+
# Add explicit tests for shellsort variants with empty vectors
267+
expect_identical(bit::shellsort(x_empty), 0L)
268+
expect_identical(bit::shellsort(x_empty, decreasing = TRUE), 0L)
269+
expect_identical(bit::shellsortorder(x_empty, integer(0)), 0L)
270+
expect_identical(bit::shellsortorder(x_empty, integer(0), decreasing = TRUE), 0L)
271+
expect_identical(bit::shellorder(x_empty, integer(0)), 0L)
272+
expect_identical(bit::shellorder(x_empty, integer(0), decreasing = TRUE), 0L)
264273

265274
# Case 2: Single Element
266275
x_single = as.integer64(1L)

0 commit comments

Comments
 (0)