From cddc65750509ba66925c1421800306e0d929b6c9 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 19:06:14 +0100 Subject: [PATCH] Use min/max values of correct type in numeric validators Use the actual type of the value, not LongestValueType, for storing m_min and m_max as this is necessary for the comparisons between the value and them to work correctly for unsigned types. Also check for precision loss when converting from the bigger LongestValueType to the actual type. Add new unit tests, that failed before, but pass now. --- include/wx/valnum.h | 80 +++++++++++++++++++------------------ tests/validators/valnum.cpp | 29 ++++++++++++++ 2 files changed, 71 insertions(+), 38 deletions(-) diff --git a/include/wx/valnum.h b/include/wx/valnum.h index 76386debf4..5305796edd 100644 --- a/include/wx/valnum.h +++ b/include/wx/valnum.h @@ -159,22 +159,22 @@ public: void SetMin(ValueType min) { - this->DoSetMin(static_cast(min)); + m_min = min; } ValueType GetMin() const { - return static_cast(this->DoGetMin()); + return m_min; } void SetMax(ValueType max) { - this->DoSetMax(static_cast(max)); + m_max = max; } ValueType GetMax() const { - return static_cast(this->DoGetMax()); + return m_max; } void SetRange(ValueType min, ValueType max) @@ -243,6 +243,9 @@ protected: : wxString(); } + virtual bool CanBeNegative() const wxOVERRIDE { return m_min < 0; } + + // This member is protected because it can be useful to the derived classes // in their Transfer{From,To}Window() implementations. ValueType * const m_value; @@ -266,6 +269,8 @@ private: return s; } + // Minimal and maximal values accepted (inclusive). + ValueType m_min, m_max; wxDECLARE_NO_ASSIGN_CLASS(wxNumValidator); }; @@ -305,23 +310,12 @@ protected: wxString ToString(LongestValueType value) const; bool FromString(const wxString& s, LongestValueType *value) const; - void DoSetMin(LongestValueType min) { m_min = min; } - LongestValueType DoGetMin() const { return m_min; } - void DoSetMax(LongestValueType max) { m_max = max; } - LongestValueType DoGetMax() const { return m_max; } - - bool IsInRange(LongestValueType value) const - { - return m_min <= value && value <= m_max; - } + virtual bool IsInRange(LongestValueType value) const = 0; // Implement wxNumValidatorBase pure virtual method. virtual bool IsCharOk(const wxString& val, int pos, wxChar ch) const wxOVERRIDE; private: - // Minimal and maximal values accepted (inclusive). - LongestValueType m_min, m_max; - wxDECLARE_NO_ASSIGN_CLASS(wxIntegerValidatorBase); }; @@ -337,6 +331,8 @@ public: typedef wxPrivate::wxNumValidator Base; + typedef + wxIntegerValidatorBase::LongestValueType LongestValueType; // Ctor for an integer validator. // @@ -345,14 +341,29 @@ public: wxIntegerValidator(ValueType *value = NULL, int style = wxNUM_VAL_DEFAULT) : Base(value, style) { - this->DoSetMin(std::numeric_limits::min()); - this->DoSetMax(std::numeric_limits::max()); + this->SetMin(std::numeric_limits::min()); + this->SetMax(std::numeric_limits::max()); } virtual wxObject *Clone() const wxOVERRIDE { return new wxIntegerValidator(*this); } -protected: - virtual bool CanBeNegative() const wxOVERRIDE { return DoGetMin() < 0; } + virtual bool IsInRange(LongestValueType value) const wxOVERRIDE + { + // LongestValueType is used as a container for the values of any type + // which can be used in type-independent wxIntegerValidatorBase code, + // but we need to use the correct type for comparisons, notably for + // comparing unsigned values correctly, so cast to this type and check + // that we don't lose precision while doing it. + const ValueType valueT = static_cast(value); + if ( static_cast(valueT) != value ) + { + // The conversion wasn't lossless, so the value must not be exactly + // representable in this type and so is definitely not in range. + return false; + } + + return this->GetMin() <= valueT && valueT <= this->GetMax(); + } private: wxDECLARE_NO_ASSIGN_CLASS(wxIntegerValidator); @@ -404,15 +415,7 @@ protected: wxString ToString(LongestValueType value) const; bool FromString(const wxString& s, LongestValueType *value) const; - void DoSetMin(LongestValueType min) { m_min = min; } - LongestValueType DoGetMin() const { return m_min; } - void DoSetMax(LongestValueType max) { m_max = max; } - LongestValueType DoGetMax() const { return m_max; } - - bool IsInRange(LongestValueType value) const - { - return m_min <= value && value <= m_max; - } + virtual bool IsInRange(LongestValueType value) const = 0; // Implement wxNumValidatorBase pure virtual method. virtual bool IsCharOk(const wxString& val, int pos, wxChar ch) const wxOVERRIDE; @@ -424,9 +427,6 @@ private: // Factor applied for the displayed the value. double m_factor; - // Minimal and maximal values accepted (inclusive). - LongestValueType m_min, m_max; - wxDECLARE_NO_ASSIGN_CLASS(wxFloatingPointValidatorBase); }; @@ -439,6 +439,8 @@ class wxFloatingPointValidator public: typedef T ValueType; typedef wxPrivate::wxNumValidator Base; + typedef wxFloatingPointValidatorBase::LongestValueType LongestValueType; + // Ctor using implicit (maximal) precision for this type. wxFloatingPointValidator(ValueType *value = NULL, @@ -466,19 +468,21 @@ public: return new wxFloatingPointValidator(*this); } -protected: - virtual bool CanBeNegative() const wxOVERRIDE { return DoGetMin() < 0; } + virtual bool IsInRange(LongestValueType value) const wxOVERRIDE + { + const ValueType valueT = static_cast(value); + + return this->GetMin() <= valueT && valueT <= this->GetMax(); + } private: - typedef typename Base::LongestValueType LongestValueType; - void DoSetMinMax() { // NB: Do not use min(), it's not the smallest representable value for // the floating point types but rather the smallest representable // positive value. - this->DoSetMin(static_cast(-std::numeric_limits::max())); - this->DoSetMax(static_cast( std::numeric_limits::max())); + this->SetMin(-std::numeric_limits::max()); + this->SetMax( std::numeric_limits::max()); } }; diff --git a/tests/validators/valnum.cpp b/tests/validators/valnum.cpp index c6c24eb635..8af177f3c9 100644 --- a/tests/validators/valnum.cpp +++ b/tests/validators/valnum.cpp @@ -96,6 +96,13 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferUnsigned", "[valnum]") CHECK( valUnsigned.TransferFromWindow() ); CHECK( value == 234 ); + m_text->ChangeValue("4294967295"); // == ULONG_MAX in 32 bits + CHECK( valUnsigned.TransferFromWindow() ); + CHECK( value == wxUINT32_MAX ); + + m_text->ChangeValue("4294967296"); // == ULONG_MAX + 1 + CHECK( !valUnsigned.TransferFromWindow() ); + m_text->ChangeValue("18446744073709551616"); // == ULLONG_MAX + 1 CHECK( !valUnsigned.TransferFromWindow() ); @@ -103,6 +110,28 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferUnsigned", "[valnum]") CHECK( !valUnsigned.TransferFromWindow() ); } +TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferULL", "[valnum]") +{ + unsigned long long value = 0; + wxIntegerValidator valULL(&value); + valULL.SetWindow(m_text); + + m_text->ChangeValue("9223372036854775807"); // == LLONG_MAX + CHECK( valULL.TransferFromWindow() ); + CHECK( value == static_cast(wxINT64_MAX) ); + + m_text->ChangeValue("9223372036854775808"); // == LLONG_MAX + 1 + CHECK( valULL.TransferFromWindow() ); + CHECK( value == static_cast(wxINT64_MAX) + 1 ); + + m_text->ChangeValue("18446744073709551615"); // == ULLONG_MAX + CHECK( valULL.TransferFromWindow() ); + CHECK( value == wxUINT64_MAX ); + + m_text->ChangeValue("18446744073709551616"); // == ULLONG_MAX + 1 + CHECK( !valULL.TransferFromWindow() ); +} + TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferFloat", "[valnum]") { // We need a locale with point as decimal separator.