Enable more compiler warnings (#480)

* Enable more compiler warnings

* Fix some more issues (thanks gcc)

* Fix some more implicit conversions

* Fix docs

* List them individually + more

* Remove -Wvla

* Remove -Wconditional-uninitialized because gcc on Windows is old

src/Makefile

@@ -20,7 +20,67 @@ ifeq ($(PLATFORM),Darwin)
 endif
 
 # The base compiler flags. More can be added on command line.
-CFLAGS += -I. -I../inc -O2 -Wall -Wextra
+CFLAGS += -I. -I../inc -O2
+# Enable a bunch of optional warnings.
+CFLAGS += \
+	-pedantic \
+	-Wall \
+	-Wextra \
+	-Waggregate-return \
+	-Walloca \
+	-Warray-bounds \
+	-Wbad-function-cast \
+	-Wc++-compat \
+	-Wc++11-compat \
+	-Wcast-align \
+	-Wcast-qual \
+	-Wconversion \
+	-Wdisabled-optimization \
+	-Wdouble-promotion \
+	-Wenum-compare \
+	-Wfloat-equal \
+	-Wformat-nonliteral \
+	-Wformat-security \
+	-Wformat-y2k \
+	-Wformat=2 \
+	-Wframe-larger-than=1048576 \
+	-Wimplicit \
+	-Wimplicit-fallthrough \
+	-Winit-self \
+	-Winline \
+	-Winvalid-pch \
+	-Wlarger-than=512 \
+	-Wlong-long \
+	-Wmissing-declarations \
+	-Wmissing-field-initializers \
+	-Wmissing-format-attribute \
+	-Wmissing-include-dirs \
+	-Wmissing-noreturn \
+	-Wmissing-prototypes \
+	-Wnested-externs \
+	-Wnull-dereference \
+	-Wold-style-definition \
+	-Woverlength-strings \
+	-Wpacked \
+	-Wpadded \
+	-Wpointer-arith \
+	-Wredundant-decls \
+	-Wredundant-move \
+	-Wshadow \
+	-Wsign-compare \
+	-Wsign-conversion \
+	-Wstack-protector \
+	-Wstrict-aliasing=2 \
+	-Wstrict-overflow=5 \
+	-Wstrict-prototypes \
+	-Wswitch-default \
+	-Wswitch-enum \
+	-Wtype-limits \
+	-Wundef \
+	-Wuninitialized \
+	-Wunreachable-code \
+	-Wvariadic-macros \
+	-Wwrite-strings
 
 # Cross-platform compilation settings.
 ifeq ($(PLATFORM),Windows)

src/common/lincomb.c

@@ -63,7 +63,7 @@ void g1_lincomb_naive(g1_t *out, const g1_t *p, const fr_t *coeffs, uint64_t len
  */
 C_KZG_RET g1_lincomb_fast(g1_t *out, const g1_t *p, const fr_t *coeffs, size_t len) {
     C_KZG_RET ret;
-    void *scratch = NULL;
+    limb_t *scratch = NULL;
     blst_p1 *p_filtered = NULL;
     blst_p1_affine *p_affine = NULL;
     blst_scalar *scalars = NULL;
@@ -88,7 +88,7 @@ C_KZG_RET g1_lincomb_fast(g1_t *out, const g1_t *p, const fr_t *coeffs, size_t l
 
     /* Allocate space for Pippenger scratch */
     size_t scratch_size = blst_p1s_mult_pippenger_scratch_sizeof(len);
-    ret = c_kzg_malloc(&scratch, scratch_size);
+    ret = c_kzg_malloc((void **)&scratch, scratch_size);
     if (ret != C_KZG_OK) goto out;
 
     /* Transform the field elements to 256-bit scalars */

src/common/utils.c

@@ -17,6 +17,7 @@
 #include "common/utils.h"
 #include "common/alloc.h"
 
+#include <stddef.h> /* For size_t */
 #include <stdlib.h> /* For NULL */
 #include <string.h> /* For memcpy */
 
@@ -43,26 +44,26 @@ bool is_power_of_two(uint64_t n) {
  * @return The log base two of n.
  *
  * @remark In other words, the bit index of the one bit.
- * @remark Works only for n a power of two, and only for n up to 2^31.
+ * @remark Works only for n a power of two.
  * @remark Not the fastest implementation, but it doesn't need to be fast.
  */
-int log2_pow2(uint32_t n) {
-    int position = 0;
+uint64_t log2_pow2(uint64_t n) {
+    uint64_t position = 0;
     while (n >>= 1)
         position++;
     return position;
 }
 
 /**
- * Reverse the bit order in a 32-bit integer.
+ * Reverse the bit order in a 64-bit integer.
  *
  * @param[in]   n   The integer to be reversed
  *
  * @return An integer with the bits of `n` reversed.
  */
-uint32_t reverse_bits(uint32_t n) {
-    uint32_t result = 0;
-    for (int i = 0; i < 32; ++i) {
+uint64_t reverse_bits(uint64_t n) {
+    uint64_t result = 0;
+    for (size_t i = 0; i < 64; ++i) {
         result <<= 1;
         result |= (n & 1);
         n >>= 1;
@@ -71,7 +72,7 @@ uint32_t reverse_bits(uint32_t n) {
 }
 
 /**
- * Reverse the low-order bits in a 32-bit integer.
+ * Reverse the low-order bits in a 64-bit integer.
  *
  * @param[in]   n       To reverse `b` bits, set `n = 2 ^ b`
  * @param[in]   value   The bits to be reversed
@@ -80,8 +81,8 @@ uint32_t reverse_bits(uint32_t n) {
  *
  * @remark n must be a power of two.
  */
-uint32_t reverse_bits_limited(uint32_t n, uint32_t value) {
-    size_t unused_bit_len = 32 - log2_pow2(n);
+uint64_t reverse_bits_limited(uint64_t n, uint64_t value) {
+    size_t unused_bit_len = 64 - log2_pow2(n);
     return reverse_bits(value) >> unused_bit_len;
 }
 
@@ -90,8 +91,7 @@ uint32_t reverse_bits_limited(uint32_t n, uint32_t value) {
  *
  * @param[in,out] values The array, which is re-ordered in-place
  * @param[in]     size   The size in bytes of an element of the array
- * @param[in]     n      The length of the array, must be a power of two
- *                       strictly greater than 1 and less than 2^32.
+ * @param[in]     n      The length of the array, must be a power of two strictly greater than 1
  *
  * @remark Operates in-place on the array.
  * @remark Can handle arrays of any type: provide the element size in `size`.
@@ -102,10 +102,10 @@ uint32_t reverse_bits_limited(uint32_t n, uint32_t value) {
 C_KZG_RET bit_reversal_permutation(void *values, size_t size, uint64_t n) {
     C_KZG_RET ret;
     byte *tmp = NULL;
-    byte *v = values;
+    byte *v = (byte *)values;
 
     /* Some sanity checks */
-    if (n < 2 || n >= UINT32_MAX || !is_power_of_two(n)) {
+    if (n < 2 || !is_power_of_two(n)) {
         ret = C_KZG_BADARGS;
         goto out;
     }
@@ -115,9 +115,9 @@ C_KZG_RET bit_reversal_permutation(void *values, size_t size, uint64_t n) {
     if (ret != C_KZG_OK) goto out;
 
     /* Reorder elements */
-    int unused_bit_len = 32 - log2_pow2(n);
-    for (uint32_t i = 0; i < n; i++) {
-        uint32_t r = reverse_bits(i) >> unused_bit_len;
+    uint64_t unused_bit_len = 64 - log2_pow2(n);
+    for (uint64_t i = 0; i < n; i++) {
+        uint64_t r = reverse_bits(i) >> unused_bit_len;
         if (r > i) {
             /* Swap the two elements */
             memcpy(tmp, v + (i * size), size);
@@ -140,9 +140,9 @@ C_KZG_RET bit_reversal_permutation(void *values, size_t size, uint64_t n) {
  *
  * @remark `out` is left untouched if `n == 0`.
  */
-void compute_powers(fr_t *out, const fr_t *x, uint64_t n) {
+void compute_powers(fr_t *out, const fr_t *x, size_t n) {
     fr_t current_power = FR_ONE;
-    for (uint64_t i = 0; i < n; i++) {
+    for (size_t i = 0; i < n; i++) {
         out[i] = current_power;
         blst_fr_mul(&current_power, &current_power, x);
     }

src/common/utils.h

@@ -32,11 +32,11 @@ extern "C" {
 #endif
 
 bool is_power_of_two(uint64_t n);
-int log2_pow2(uint32_t n);
-uint32_t reverse_bits(uint32_t n);
-uint32_t reverse_bits_limited(uint32_t n, uint32_t value);
+uint64_t log2_pow2(uint64_t n);
+uint64_t reverse_bits(uint64_t n);
+uint64_t reverse_bits_limited(uint64_t n, uint64_t value);
 C_KZG_RET bit_reversal_permutation(void *values, size_t size, uint64_t n);
-void compute_powers(fr_t *out, const fr_t *x, uint64_t n);
+void compute_powers(fr_t *out, const fr_t *x, size_t n);
 bool pairings_verify(const g1_t *a1, const g2_t *a2, const g1_t *b1, const g2_t *b2);
 
 #ifdef __cplusplus

src/eip4844/blob.c

@@ -29,7 +29,7 @@
 C_KZG_RET blob_to_polynomial(fr_t *p, const Blob *blob) {
     C_KZG_RET ret;
     for (size_t i = 0; i < FIELD_ELEMENTS_PER_BLOB; i++) {
-        ret = bytes_to_bls_field(&p[i], (Bytes32 *)&blob->bytes[i * BYTES_PER_FIELD_ELEMENT]);
+        ret = bytes_to_bls_field(&p[i], (const Bytes32 *)&blob->bytes[i * BYTES_PER_FIELD_ELEMENT]);
         if (ret != C_KZG_OK) return ret;
     }
     return C_KZG_OK;
@@ -42,7 +42,7 @@ C_KZG_RET blob_to_polynomial(fr_t *p, const Blob *blob) {
  */
 void print_blob(const Blob *blob) {
     for (size_t i = 0; i < FIELD_ELEMENTS_PER_BLOB; i++) {
-        Bytes32 *field = (Bytes32 *)&blob->bytes[i * BYTES_PER_FIELD_ELEMENT];
+        const Bytes32 *field = (const Bytes32 *)&blob->bytes[i * BYTES_PER_FIELD_ELEMENT];
         print_bytes32(field);
     }
 }

src/eip7594/cell.c

@@ -26,7 +26,7 @@
  */
 void print_cell(const Cell *cell) {
     for (size_t i = 0; i < FIELD_ELEMENTS_PER_CELL; i++) {
-        Bytes32 *field = (Bytes32 *)&cell->bytes[i * BYTES_PER_FIELD_ELEMENT];
+        const Bytes32 *field = (const Bytes32 *)&cell->bytes[i * BYTES_PER_FIELD_ELEMENT];
         print_bytes32(field);
     }
 }

src/eip7594/eip7594.c

@@ -224,7 +224,7 @@ C_KZG_RET recover_cells_and_kzg_proofs(
 
             /* Convert the untrusted bytes to a field element */
             size_t offset = j * BYTES_PER_FIELD_ELEMENT;
-            ret = bytes_to_bls_field(field, (Bytes32 *)&cells[i].bytes[offset]);
+            ret = bytes_to_bls_field(field, (const Bytes32 *)&cells[i].bytes[offset]);
             if (ret != C_KZG_OK) goto out;
         }
     }
@@ -632,7 +632,7 @@ C_KZG_RET verify_cell_kzg_proof_batch(
         for (size_t j = 0; j < FIELD_ELEMENTS_PER_CELL; j++) {
             fr_t field, scaled;
             size_t offset = j * BYTES_PER_FIELD_ELEMENT;
-            ret = bytes_to_bls_field(&field, (Bytes32 *)&cells[i].bytes[offset]);
+            ret = bytes_to_bls_field(&field, (const Bytes32 *)&cells[i].bytes[offset]);
             if (ret != C_KZG_OK) goto out;
             blst_fr_mul(&scaled, &field, &r_powers[i]);
             size_t index = cell_indices[i] * FIELD_ELEMENTS_PER_CELL + j;
@@ -680,7 +680,7 @@ C_KZG_RET verify_cell_kzg_proof_batch(
          * To unscale, divide by the coset. It's faster to multiply with the inverse. We can skip
          * the first iteration because its dividing by one.
          */
-        uint32_t pos = reverse_bits_limited(CELLS_PER_EXT_BLOB, i);
+        uint64_t pos = reverse_bits_limited(CELLS_PER_EXT_BLOB, i);
         fr_t inv_coset_factor;
         blst_fr_eucl_inverse(&inv_coset_factor, &s->roots_of_unity[pos]);
         shift_poly(column_interpolation_poly, FIELD_ELEMENTS_PER_CELL, &inv_coset_factor);
@@ -709,7 +709,7 @@ C_KZG_RET verify_cell_kzg_proof_batch(
     ////////////////////////////////////////////////////////////////////////////////////////////////
 
     for (size_t i = 0; i < num_cells; i++) {
-        uint32_t pos = reverse_bits_limited(CELLS_PER_EXT_BLOB, cell_indices[i]);
+        uint64_t pos = reverse_bits_limited(CELLS_PER_EXT_BLOB, cell_indices[i]);
         fr_t coset_factor = s->roots_of_unity[pos];
         fr_pow(&weights[i], &coset_factor, FIELD_ELEMENTS_PER_CELL);
         blst_fr_mul(&weighted_powers_of_r[i], &r_powers[i], &weights[i]);

src/eip7594/fk20.c

@@ -73,7 +73,7 @@ C_KZG_RET compute_fk20_proofs(g1_t *out, const fr_t *p, size_t n, const KZGSetti
     fr_t *toeplitz_coeffs_fft = NULL;
     g1_t *h = NULL;
     g1_t *h_ext_fft = NULL;
-    void *scratch = NULL;
+    limb_t *scratch = NULL;
     bool precompute = s->wbits != 0;
 
     /* Initialize length variables */
@@ -92,7 +92,7 @@ C_KZG_RET compute_fk20_proofs(g1_t *out, const fr_t *p, size_t n, const KZGSetti
 
     if (precompute) {
         /* Allocations for fixed-base MSM */
-        ret = c_kzg_malloc(&scratch, s->scratch_size);
+        ret = c_kzg_malloc((void **)&scratch, s->scratch_size);
         if (ret != C_KZG_OK) goto out;
         ret = c_kzg_calloc((void **)&scalars, FIELD_ELEMENTS_PER_CELL, sizeof(blst_scalar));
         if (ret != C_KZG_OK) goto out;

src/eip7594/poly.c

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include "poly.h"
 #include "common/alloc.h"
 #include "common/ec.h"
 #include "common/ret.h"

src/eip7594/recovery.c

@@ -252,7 +252,7 @@ C_KZG_RET recover_cells(
         /* Iterate over each cell index and check if we have received it */
         if (!is_in_array(cell_indices, num_cells, i)) {
             /* If the cell is missing, bit reverse the index and add it to the missing array */
-            uint32_t brp_i = reverse_bits_limited(CELLS_PER_EXT_BLOB, i);
+            uint64_t brp_i = reverse_bits_limited(CELLS_PER_EXT_BLOB, i);
             missing_cell_indices[len_missing++] = brp_i;
         }
     }

src/setup/setup.c

@@ -94,7 +94,7 @@ static C_KZG_RET compute_roots_of_unity(KZGSettings *s) {
     C_KZG_RET ret;
     fr_t root_of_unity;
 
-    uint32_t max_scale = 0;
+    size_t max_scale = 0;
     while ((1ULL << max_scale) < FIELD_ELEMENTS_PER_EXT_BLOB)
         max_scale++;
 
@@ -382,7 +382,7 @@ C_KZG_RET load_trusted_setup(
     }
 
     /* 1<<max_scale is the smallest power of 2 >= n1 */
-    uint32_t max_scale = 0;
+    size_t max_scale = 0;
     while ((1ULL << max_scale) < NUM_G1_POINTS)
         max_scale++;