From ee84db4c18562c5dfeb1e7cf2d9fb1fcd4490dfa Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Wed, 27 Sep 2017 11:14:11 +0200 Subject: [PATCH] QLocaleData::longLongToString: clean up sign handling In qlltoa we simply discarded sign, passing absolute value down to qulltoa; given that it had just one caller and did something only that caller is apt to want - violating the principle of least surprise for those who would expect it to handle sign sanely - it is better to just inline it in the one place it was used. That one place, QLocaleData::longLongToString(), handles sign separately and (for reasons I shall argue elsewhere are bogus) ignores sign for bases other than ten. When ignoring sign, it passes negative values directly to qulltoa, which caught the attention of Coverity (CID 22326). This deliberately cast (for the same bogus reasons) negatives to large unsigned. Inlining base ten's call to qlltoa() as a call to qulltoa() could now be unified into a simple (and less obviously perverse) form. Making the (contentious) cast explicit can then calm Coverity and we get a simpler implementation all round. Incorporate also Marc Mutz's fix for unary minus applied to unsigned and handling of the most negative value, suitably adapted. Change-Id: Iea02500a5dd7c6d7ac6e73656e1b11f116ae85c6 Reviewed-by: Thiago Macieira --- src/corelib/tools/qlocale.cpp | 14 +++++++++----- src/corelib/tools/qlocale_tools.cpp | 5 ----- src/corelib/tools/qlocale_tools_p.h | 1 - 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/corelib/tools/qlocale.cpp b/src/corelib/tools/qlocale.cpp index 9a46018ede2..ebda40bd8da 100644 --- a/src/corelib/tools/qlocale.cpp +++ b/src/corelib/tools/qlocale.cpp @@ -3149,11 +3149,15 @@ QString QLocaleData::longLongToString(const QChar zero, const QChar group, negative = false; // neither are negative numbers } - QString num_str; - if (base == 10) - num_str = qlltoa(l, base, zero); - else - num_str = qulltoa(l, base, zero); +QT_WARNING_PUSH + /* "unary minus operator applied to unsigned type, result still unsigned" */ +QT_WARNING_DISABLE_MSVC(4146) + /* + Negating std::numeric_limits::min() hits undefined behavior, so + taking an absolute value has to cast to unsigned to change sign. + */ + QString num_str = qulltoa(negative ? -qulonglong(l) : qulonglong(l), base, zero); +QT_WARNING_POP uint cnt_thousand_sep = 0; if (flags & ThousandsGroup && base == 10) { diff --git a/src/corelib/tools/qlocale_tools.cpp b/src/corelib/tools/qlocale_tools.cpp index 3e4f37501e0..4d969a4723f 100644 --- a/src/corelib/tools/qlocale_tools.cpp +++ b/src/corelib/tools/qlocale_tools.cpp @@ -456,11 +456,6 @@ QString qulltoa(qulonglong l, int base, const QChar _zero) return QString(reinterpret_cast(p), 65 - (p - buff)); } -QString qlltoa(qlonglong l, int base, const QChar zero) -{ - return qulltoa(l < 0 ? -l : l, base, zero); -} - QString &decimalForm(QChar zero, QChar decimal, QChar group, QString &digits, int decpt, int precision, PrecisionMode pm, diff --git a/src/corelib/tools/qlocale_tools_p.h b/src/corelib/tools/qlocale_tools_p.h index 6133f67add1..742abb49578 100644 --- a/src/corelib/tools/qlocale_tools_p.h +++ b/src/corelib/tools/qlocale_tools_p.h @@ -83,7 +83,6 @@ void doubleToAscii(double d, QLocaleData::DoubleForm form, int precision, char * bool &sign, int &length, int &decpt); QString qulltoa(qulonglong l, int base, const QChar _zero); -QString qlltoa(qlonglong l, int base, const QChar zero); Q_CORE_EXPORT QString qdtoa(qreal d, int *decpt, int *sign); enum PrecisionMode {