fixup! hsort

Co-authored-by: Tim Ruffing <crypto@timruffing.de>
Co-authored-by: Russell O'Connor <roconnor@blockstream.io>

Author: 3 people

src/hsort_impl.h

@@ -23,23 +23,26 @@ static SECP256K1_INLINE size_t secp256k1_heap_child2(size_t i) {
     return secp256k1_heap_child1(i)+1;
 }
 
-static SECP256K1_INLINE void secp256k1_heap_swap64(unsigned char *a, size_t i, size_t j, size_t stride) {
+static SECP256K1_INLINE void secp256k1_heap_swap64(unsigned char *a, unsigned char *b, size_t len) {
     unsigned char tmp[64];
-    VERIFY_CHECK(stride <= 64);
-    memcpy(tmp, a + i*stride, stride);
-    memmove(a + i*stride, a + j*stride, stride);
-    memcpy(a + j*stride, tmp, stride);
+    VERIFY_CHECK(len <= 64);
+    memcpy(tmp, a, len);
+    memmove(a, b, len);
+    memcpy(b, tmp, len);
 }
 
-static SECP256K1_INLINE void secp256k1_heap_swap(unsigned char *a, size_t i, size_t j, size_t stride) {
-    while (64 < stride) {
-        secp256k1_heap_swap64(a + (stride - 64), i, j, 64);
-        stride -= 64;
+static SECP256K1_INLINE void secp256k1_heap_swap(unsigned char *arr, size_t i, size_t j, size_t stride) {
+    unsigned char *a = arr + i*stride;
+    unsigned char *b = arr + j*stride;
+    size_t len = stride;
+    while (64 < len) {
+        secp256k1_heap_swap64(a + (len - 64), b + (len - 64), 64);
+        len -= 64;
     }
-    secp256k1_heap_swap64(a, i, j, stride);
+    secp256k1_heap_swap64(a, b, len);
 }
 
-static SECP256K1_INLINE void secp256k1_heap_down(unsigned char *a, size_t i, size_t heap_size, size_t stride,
+static SECP256K1_INLINE void secp256k1_heap_down(unsigned char *arr, size_t i, size_t heap_size, size_t stride,
                             int (*cmp)(const void *, const void *, void *), void *cmp_data) {
     while (i < heap_size/2) {
         VERIFY_CHECK(i <= SIZE_MAX/2 - 1);
@@ -69,18 +72,18 @@ static SECP256K1_INLINE void secp256k1_heap_down(unsigned char *a, size_t i, siz
          * Else if [child2(i)] > [i], swap [i] with [child2(i)].
          */
         if (secp256k1_heap_child2(i) < heap_size
-                && 0 <= cmp(a + secp256k1_heap_child2(i)*stride, a + secp256k1_heap_child1(i)*stride, cmp_data)) {
-            if (0 < cmp(a + secp256k1_heap_child2(i)*stride, a +         i*stride, cmp_data)) {
-                secp256k1_heap_swap(a, i, secp256k1_heap_child2(i), stride);
+                && 0 <= cmp(arr + secp256k1_heap_child2(i)*stride, arr + secp256k1_heap_child1(i)*stride, cmp_data)) {
+            if (0 < cmp(arr + secp256k1_heap_child2(i)*stride, arr +         i*stride, cmp_data)) {
+                secp256k1_heap_swap(arr, i, secp256k1_heap_child2(i), stride);
                 i = secp256k1_heap_child2(i);
             } else {
                 /* At this point we have [child2(i)] >= [child1(i)] and we have
                  * [child2(i)] <= [i], and thus [child1(i)] <= [i] which means
                  * that the next comparison can be skipped. */
                 return;
             }
-        } else if (0 < cmp(a + secp256k1_heap_child1(i)*stride, a +         i*stride, cmp_data)) {
-            secp256k1_heap_swap(a, i, secp256k1_heap_child1(i), stride);
+        } else if (0 < cmp(arr + secp256k1_heap_child1(i)*stride, arr +         i*stride, cmp_data)) {
+            secp256k1_heap_swap(arr, i, secp256k1_heap_child1(i), stride);
             i = secp256k1_heap_child1(i);
         } else {
             return;

src/tests.c

@@ -6607,6 +6607,18 @@ static void run_pubkey_comparison(void) {
     CHECK(secp256k1_ec_pubkey_cmp(CTX, &pk2, &pk1) > 0);
 }
 
+static void test_heap_swap(void) {
+    unsigned char a[600];
+    unsigned char e[sizeof(a)];
+    memset(a,       21, 200);
+    memset(a + 200, 99, 200);
+    memset(a + 400, 42, 200);
+    memset(e,       42, 200);
+    memset(e + 200, 99, 200);
+    memset(e + 400, 21, 200);
+    secp256k1_heap_swap(a, 0, 2, 200);
+    CHECK(secp256k1_memcmp_var(a, e, sizeof(a)) == 0);
+}
 
 static void test_hsort_is_sorted(int *ints, size_t n) {
     size_t i;
@@ -6801,6 +6813,7 @@ static void run_pubkey_sort(void) {
     test_sort_api();
     test_sort();
     test_sort_vectors();
+    test_heap_swap();
 }