QString::toShort problem

kshegunov

I thought you might be Russian: are you "Mafiosa", could you send some "heavies" round to @jsulm for me...?

No, I'm not russian, but let's assume I know a guy who knows a guy, who knows a guy ... ;)

What is it in the implementation of qint64 QString::toLongLong() which makes this set ok=false instead of returning -2 ?

https://code.woboq.org/qt5/qtbase/src/corelib/tools/qlocale.cpp.html#_ZNK11QLocaleData15numberToCLocaleE11QStringView6QFlagsIN7QLocale12NumberOptionEEP15QVarLengthArrayIcLi256EE

JonB

@kshegunov
The link you provide, for QLocaleData::numberToCLocale(), is for outputting internal numbers -> external strings. For QString::toLongLong() I am looking for function code which is for inputting external string -> internal number, and see why 0xFFFFFFFFFFFFFFFE is causing a conversion error?

kshegunov

It's part of the implementation - the checking part. Sorry forgot the first link x_x.
See here:
https://code.woboq.org/qt5/qtbase/src/corelib/tools/qlocale.cpp.html#_ZNK11QLocaleData16stringToLongLongE11QStringViewiPb6QFlagsIN7QLocale12NumberOptionEE
and here:
https://code.woboq.org/qt5/qtbase/src/corelib/tools/qlocale.cpp.html#_ZN11QLocaleData19bytearrayToLongLongEPKciPb

J.Hilk

@JonB
no, @kshegunov is right,

toLongLong calls, stringtolonglong

qlonglong QLocaleData::stringToLongLong(QStringView str, int base, bool *ok,
                                        QLocale::NumberOptions number_options) const
{
    CharBuff buff;
    if (!numberToCLocale(str, number_options, &buff)) {
        if (ok != 0)
            *ok = false;
        return 0;
    }
    return bytearrayToLongLong(buff.constData(), base, ok);
}

that calls numberToCLocale

and uses the resulting char array to call bytearrayToLongLong


qlonglong QLocaleData::bytearrayToLongLong(const char *num, int base, bool *ok)
{
    bool _ok;
    const char *endptr;
    if (*num == '\0') {
        if (ok != 0)
            *ok = false;
        return 0;
    }
    qlonglong l = qstrtoll(num, &endptr, base, &_ok);
    if (!_ok) {
        if (ok != 0)
            *ok = false;
        return 0;
    }
    if (*endptr != '\0') {
        // we stopped at a non-digit character after converting some digits
        if (ok != 0)
            *ok = false;
        return 0;
    }
    if (ok != 0)
        *ok = true;
    return l;
}

long long
qstrtoll(const char * nptr, const char **endptr, int base, bool *ok)
{
    *ok = true;
    errno = 0;
    char *endptr2 = 0;
    long long result = qt_strtoll(nptr, &endptr2, base);
    if (endptr)
        *endptr = endptr2;
    if ((result == 0 || result == std::numeric_limits<long long>::min()
         || result == std::numeric_limits<long long>::max())
            && (errno || nptr == endptr2)) {
        *ok = false;
        return 0;
    }
    return result;
}

like I said, a rRabbit Hole

JonB

@J.Hilk
OK, in that case, what's the implementation of qt_strtoll(), when performed on 0xFFFFFFFFFFFFFFFE? I'm expecting it to return -2, but I'm guessing it returns std::numeric_limits<long long>::min() (or maybe ::max()), but why?

J.Hilk

@JonB said in QString::toShort problem:

@J.Hilk
OK, in that case, what's the implementation of qt_strtoll(),

I live to serve :)

/*-
 * Copyright (c) 1992, 1993
 *	The Regents of the University of California.  All rights reserved.
 *
 * Copyright (c) 2011 The FreeBSD Foundation
 * All rights reserved.
 * Portions of this software were developed by David Chisnall
 * under sponsorship from the FreeBSD Foundation.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in the
 *    documentation and/or other materials provided with the distribution.
 * 3. Neither the name of the University nor the names of its contributors
 *    may be used to endorse or promote products derived from this software
 *    without specific prior written permission.
 *
 * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
 * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
 * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
 * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 */
/*
 * Convert a string to a long long integer.
 *
 * Assumes that the upper and lower case
 * alphabets and digits are each contiguous.
 */
long long
qt_strtoll(const char * nptr, char **endptr, int base)
{
	const char *s;
	unsigned long long acc;
	char c;
	unsigned long long cutoff;
	int neg, any, cutlim;
	/*
	 * Skip white space and pick up leading +/- sign if any.
	 * If base is 0, allow 0x for hex and 0 for octal, else
	 * assume decimal; if base is already 16, allow 0x.
	 */
	s = nptr;
	do {
		c = *s++;
	} while (ascii_isspace(c));
	if (c == '-') {
		neg = 1;
		c = *s++;
	} else {
		neg = 0;
		if (c == '+')
			c = *s++;
	}
	if ((base == 0 || base == 16) &&
	    c == '0' && (*s == 'x' || *s == 'X') &&
	    ((s[1] >= '0' && s[1] <= '9') ||
	    (s[1] >= 'A' && s[1] <= 'F') ||
	    (s[1] >= 'a' && s[1] <= 'f'))) {
		c = s[1];
		s += 2;
		base = 16;
	}
	if (base == 0)
		base = c == '0' ? 8 : 10;
	acc = any = 0;
	if (base < 2 || base > 36)
		goto noconv;
	/*
	 * Compute the cutoff value between legal numbers and illegal
	 * numbers.  That is the largest legal value, divided by the
	 * base.  An input number that is greater than this value, if
	 * followed by a legal input character, is too big.  One that
	 * is equal to this value may be valid or not; the limit
	 * between valid and invalid numbers is then based on the last
	 * digit.  For instance, if the range for quads is
	 * [-9223372036854775808..9223372036854775807] and the input base
	 * is 10, cutoff will be set to 922337203685477580 and cutlim to
	 * either 7 (neg==0) or 8 (neg==1), meaning that if we have
	 * accumulated a value > 922337203685477580, or equal but the
	 * next digit is > 7 (or 8), the number is too big, and we will
	 * return a range error.
	 *
	 * Set 'any' if any `digits' consumed; make it negative to indicate
	 * overflow.
	 */
	cutoff = neg ? (unsigned long long)-(LLONG_MIN + LLONG_MAX) + LLONG_MAX
	    : LLONG_MAX;
	cutlim = cutoff % base;
	cutoff /= base;
	for ( ; ; c = *s++) {
		if (c >= '0' && c <= '9')
			c -= '0';
		else if (c >= 'A' && c <= 'Z')
			c -= 'A' - 10;
		else if (c >= 'a' && c <= 'z')
			c -= 'a' - 10;
		else
			break;
		if (c >= base)
			break;
		if (any < 0 || acc > cutoff || (acc == cutoff && c > cutlim))
			any = -1;
		else {
			any = 1;
			acc *= base;
			acc += c;
		}
	}
	if (any < 0) {
		acc = neg ? LLONG_MIN : LLONG_MAX;
		errno = ERANGE;
	} else if (!any) {
noconv:
		errno = EINVAL;
	} else if (neg)
		acc = (unsigned long long) -(long long)acc;
	if (endptr != NULL)
                *endptr = const_cast<char *>(any ? s - 1 : nptr);
	return (acc);
}

that said, here, my goto online reference, for future selfresearch:
https://code.woboq.org/qt5/
this webside has the benefit of mouse navigation (right click) to functions and their declartions/defininations

JonB

@J.Hilk
Thanks! So --- not that I claim to understand the code --- the ultimate reason 0xFFFFFFFFFFFFFFFE errors as a qint64 instead of returning -2 must be the long comment about "Compute the cutoff value between legal numbers and illegal numbers.", the computation of cutoff & cutlim , and the test fragment acc > cutoff || (acc == cutoff && c > cutlim).

J.Hilk

@JonB said in QString::toShort problem:

@J.Hilk
Thanks! So --- not that I claim to understand the code --- the ultimate reason 0xFFFFFFFFFFFFFFFE errors as a qint64 instead of returning -2 must be the long comment about "Compute the cutoff value between legal numbers and illegal numbers.", the computation of cutoff & cutlim , and the test fragment acc > cutoff || (acc == cutoff && c > cutlim).

Actually, I would consider this a bug, the "cutoff seems to happen one char to early

    qDebug() << 0xFFFFFFFFFFFFFFFF
             << static_cast<int64_t>(0xFFFFFFFFFFFFFFFF)
             << QString("0xFFFFFFFFFFFFFFFF").toLongLong(nullptr,16)
             << QString("0xFFFFFFFFFFFFFFF").toLongLong(nullptr,16)
             << QString::number(1152921504606846975,16)
             << QString::number(18446744073709551615,16)
             << QString::number(18446744073709551615,16).toLongLong(nullptr, 16);
//results in
18446744073709551615 
-1 
0 
1152921504606846975 
"fffffffffffffff" 
"ffffffffffffffff"
0

that makes QString::number(18446744073709551615,16) irreversible as QString::number(18446744073709551615,16).toLongLong(&ok, 16); result in false & 0(as value)

JonB

@J.Hilk
I wish you'd write those long decimal numbers in hex so they're easier to understand!

I find this worrying, as I'm often needing to use 18446744073709551615 in everyday life (e.g. my prediction for number of goals England will score in World Cup).

As for your finding, are you suggesting the condition should have had c >= cutlim in it?

J.Hilk

@JonB actually, its no bug,

QString("0x7FFFFFFFFFFFFFFF").toLongLong(nullptr,16) converts just fine to int64, and this happens to be the limit of int64_t

std::numeric_limits<int64_t>::max() = 0x7FFFFFFFFFFFFFFF

QString("0xFFFFFFFFFFFFFFFF").toULongLong(nullptr, 16) = 0xFFFFFFFFFFFFFFFF
The moral of the story is, you better know beforehand if your target value is signed or unsigned.

JonB

@J.Hilk said in QString::toShort problem:

The moral of the story is, you better know beforehand if your target value is signed or unsigned.

Which is just about where this whole thread started out from... :)

J.Hilk

@JonB
;-) indeed.

There are some improvements to QString, that - arguably - could be made.
For example:

QString::number(short(-2),16) = "fffffffffffffffe" = QString::number(int64_t(-2),16)

You would, eventually be aware of this, and you can truncate the returning string, but...
-_-

kshegunov

@J.Hilk said in QString::toShort problem:

You would, eventually be aware of this, and you can truncate the returning string, but...

Please file this as a bug, the correct overload (i.e. the one taking int) is called, but the QString::setNum does not care to truncate the result properly. Should be fixed, although it's a minor one.

J.Hilk

@kshegunov are you sure thats the issue?

QString QString::number(int n, int base)
{
    return number(qlonglong(n), base);
}

this seems to be the internal call, and thats the call for every overload - seems like there was more intended once.

-> when <0 allocate qulonglong size memory, fill it with max =0xF
https://code.woboq.org/qt5/qtbase/src/corelib/tools/qlocale_tools.cpp.html#427

Christian Ehrlicher

@J.Hilk said in QString::toShort problem:

this seems to be the internal call, and thats the call for every overload - seems like there was more intended once.

There is a bugreport about this but currently without any real solution:
https://bugreports.qt.io/browse/QTBUG-1098
https://bugreports.qt.io/browse/QTBUG-53706