ECC-1930: Fix more overflows

src/NumericLimits.h

@@ -11,59 +11,99 @@
 #pragma once
 
 #include <array>
+#include <cstdint>
+#include <climits> // CHAR_BIT
 
-template <typename ValueType>
+// NumericLimits is a class template that provides the minimum and maximum values for a given number of bits.
+// The minimum and maximum values are calculated for both signed and unsigned integral types.
+
+// Example:
+// For a 16-bit signed integer, the minimum and maximum values are:
+// nbits | min | max
+// ----- | --- | ---
+// 1     | -1  | 0
+// 2     | -2  | 1
+// 3     | -4  | 3
+// 4     | -8  | 7
+// ...
+// 15    | -16384 | 16383
+// 16    | -32768 | 32767
+
+template <typename ValueType, bool = std::is_signed<ValueType>::value, bool = std::is_integral<ValueType>::value>
 struct NumericLimits
 {
-    static_assert(std::is_integral<ValueType>::value, "ValueType must be an integral type");
-
 private:
-    using S                   = std::make_signed_t<ValueType>;
+    static constexpr uint8_t MaxN = CHAR_BIT * sizeof(ValueType);
-    using U                   = std::make_unsigned_t<ValueType>;
-    static constexpr int MaxN = 8 * sizeof(ValueType);
 
     static constexpr std::array<ValueType, MaxN> max_ = []() {
-        if constexpr (std::is_signed<ValueType>::value) {
+        std::array<ValueType, MaxN> max{};
-            std::array<S, MaxN> max{};
+        using UnsignedValueType          = std::make_unsigned_t<ValueType>;
-            for (int i = 1; i < MaxN; ++i) {
+        constexpr UnsignedValueType ones = ~UnsignedValueType{0};
-                auto ones = ~(static_cast<U>(0));
+        max[0]                           = 0;
-                max[i]    = static_cast<S>(ones >> (MaxN - i));
+        for (uint8_t i = 1; i < MaxN; ++i) {
-            }
+            max[i] = static_cast<ValueType>(ones >> (MaxN - i));
-            return max;
-        }
-        else {
-            std::array<U, MaxN> max{};
-            for (int i = 1; i < MaxN; ++i) {
-                auto ones = ~(static_cast<U>(0));
-                max[i]    = ones >> (MaxN - i - 1);
-            }
-            return max;
         }
+        return max;
     }();
 
     static constexpr std::array<ValueType, MaxN> min_ = []() {
-        if constexpr (std::is_signed<ValueType>::value) {
+        std::array<ValueType, MaxN> min{};
-            std::array<S, MaxN> min{};
+        for (uint8_t i = 0; i < MaxN; ++i) {
-            for (int i = 1; i < MaxN; ++i) {
+            min[i] = ~max_[i];
-                min[i] = ~max_[i];
-            }
-            return min;
-        }
-        else {
-            return std::array<U, MaxN>{};
         }
+        return min;
     }();
 
 public:
-    static constexpr ValueType min(int nbits)
+    static constexpr ValueType min(uint8_t nbits)
     {
-        if constexpr (std::is_signed<ValueType>::value)
+        return min_[nbits - 1];
-            return min_[nbits - 1];
-        else
-            return 0;
     }
 
-    static constexpr ValueType max(int nbits)
+    static constexpr ValueType max(uint8_t nbits)
+    {
+        return max_[nbits - 1];
+    }
+};
+
+
+// Example:
+// For a 16-bit unsigned integer, the minimum and maximum values are:
+// nbits | min | max
+// ----- | --- | ---
+// 1     | 0   | 1
+// 2     | 0   | 3
+// 3     | 0   | 7
+// 4     | 0   | 15
+// ...
+// 15    | 0   | 32767
+// 16    | 0   | 65535
+
+template <typename ValueType>
+struct NumericLimits<ValueType, false, true>
+{
+    static_assert(std::is_integral<ValueType>::value, "ValueType must be an integral type");
+
+private:
+    static constexpr uint8_t MaxN = CHAR_BIT * sizeof(ValueType);
+
+    static constexpr std::array<ValueType, MaxN> max_ = []() {
+        std::array<ValueType, MaxN> max{};
+        constexpr ValueType ones = ~(static_cast<ValueType>(0));
+        max[0]                   = 1;
+        for (uint8_t i = 1; i < MaxN; ++i) {
+            max[i] = ones >> (MaxN - i - 1);
+        }
+        return max;
+    }();
+
+public:
+    static constexpr ValueType min(uint8_t nbits)
+    {
+        return 0;
+    }
+
+    static constexpr ValueType max(uint8_t nbits)
     {
         return max_[nbits - 1];
     }

src/accessor/grib_accessor_class_bits.cc

@@ -9,6 +9,8 @@
  */
 
 #include "grib_accessor_class_bits.h"
+#include "NumericLimits.h"
+
 
 grib_accessor_bits_t _grib_accessor_bits{};
 grib_accessor* grib_accessor_bits = &_grib_accessor_bits;
@@ -160,7 +162,7 @@ int grib_accessor_bits_t::pack_long(const long* val, size_t* len)
     }
 #endif
 
-    maxval = (1 << length) - 1;
+    maxval = NumericLimits<unsigned long>::max(length);
     if (*val > maxval) {
         grib_context_log(h->context, GRIB_LOG_ERROR,
                          "key=%s: Trying to encode value of %ld but the maximum allowable value is %ld (number of bits=%ld)",

src/accessor/grib_accessor_class_bufr_data_array.cc

@@ -13,6 +13,7 @@
 #include "grib_accessor_class_expanded_descriptors.h"
 #include "grib_accessor_class_bufr_data_element.h"
 #include "grib_accessor_class_variable.h"
+#include "NumericLimits.h"
 
 grib_accessor_bufr_data_array_t _grib_accessor_bufr_data_array{};
 grib_accessor* grib_accessor_bufr_data_array = &_grib_accessor_bufr_data_array;
@@ -131,8 +132,8 @@ int grib_accessor_bufr_data_array_t::tableB_override_set_key(grib_handle* h)
 /* Check numBits is sufficient for entries in the overridden reference values list*/
 static int check_overridden_reference_values(const grib_context* c, long* refValList, size_t refValListSize, int numBits)
 {
-    const long maxval = (1 << (numBits - 1)) - 1;
+    const long maxval = NumericLimits<long>::max(numBits);
-    const long minval = -(1 << (numBits - 1));
+    const long minval = NumericLimits<long>::min(numBits);
     size_t i          = 0;
     for (i = 0; i < refValListSize; ++i) {
         grib_context_log(c, GRIB_LOG_DEBUG, "check_overridden_reference_values: refValList[%ld]=%ld", i, refValList[i]);

src/accessor/grib_accessor_class_from_scale_factor_scaled_value.cc

@@ -10,6 +10,7 @@
  */
 
 #include "grib_accessor_class_from_scale_factor_scaled_value.h"
+#include "NumericLimits.h"
 
 grib_accessor_from_scale_factor_scaled_value_t _grib_accessor_from_scale_factor_scaled_value{};
 grib_accessor* grib_accessor_from_scale_factor_scaled_value = &_grib_accessor_from_scale_factor_scaled_value;
@@ -64,8 +65,8 @@ int grib_accessor_from_scale_factor_scaled_value_t::pack_double(const double* va
     }
     value_accessor_num_bits  = value_accessor->length_ * 8;
     factor_accessor_num_bits = factor_accessor->length_ * 8;
-    maxval_value             = (1UL << value_accessor_num_bits) - 2;   // exclude missing
+    maxval_value             = NumericLimits<int64_t>::max(value_accessor_num_bits);  // exclude missing
-    maxval_factor            = (1UL << factor_accessor_num_bits) - 2;  // exclude missing
+    maxval_factor            = NumericLimits<int64_t>::max(factor_accessor_num_bits);  // exclude missing
     if (strcmp(factor_accessor->class_name_, "signed") == 0) {
         maxval_factor = (1UL << (factor_accessor_num_bits - 1)) - 1;
     }

src/accessor/grib_accessor_class_unsigned.cc

@@ -9,6 +9,7 @@
  */
 
 #include "grib_accessor_class_unsigned.h"
+#include "NumericLimits.h"
 
 grib_accessor_unsigned_t _grib_accessor_unsigned{};
 grib_accessor* grib_accessor_unsigned = &_grib_accessor_unsigned;
@@ -116,7 +117,7 @@ int grib_accessor_unsigned_t::pack_long_unsigned_helper(const long* val, size_t*
             if (!value_is_missing(v)) {
                 const long nbits = nbytes_ * 8;
                 if (nbits < 33) {
-                    unsigned long maxval = (1UL << nbits) - 1;
+                    unsigned long maxval = NumericLimits<unsigned long>::max(nbits);
                     if (maxval > 0 && v > maxval) { /* See ECC-1002 */
                         grib_context_log(context_, GRIB_LOG_ERROR,
                                          "Key \"%s\": Trying to encode value of %ld but the maximum allowable value is %lu (number of bits=%ld)",

tests/grib_set_fail.sh

@@ -77,7 +77,7 @@ if [ $ECCODES_ON_WINDOWS -eq 0 ]; then
     status=$?
     set -e
     [ $status -ne 0 ]
-    grep -q "Trying to encode value of 2147483648 but the allowable range is -2147483647 to 2147483647" $temp
+    grep -q "Trying to encode value of 2147483648 but the allowable range is -2147483648 to 2147483647" $temp
 
     set +e
     ${tools_dir}/grib_set -s forecastTime=-2147483650 $input $outfile 2>$temp