terminator356
commented
1 year ago Hi! In the following test code which includes both the existing and proposed new function versions, unless I've entered something wrong, there is a problem, shown after this listing:
#include <cstdio>
#include <cstdlib>
#include <cassert>
#include <vector>
#include <locale>
#include <sstream>
#include <QString>
//===========================
// Existing functions:
//===========================
QString museStringFromDouble(double v)
{
// NOTE: snprintf can be locale sensitive! We should force the locale to standard 'C'.
// Use a per-thread thread-safe technique instead of setting the global locale.
// Note this C locale is NOT the same as the application's C++ locale.
// Setting LANG environment variable before running is different than setting our -l switch.
locale_t nloc = newlocale(LC_NUMERIC_MASK, "C", (locale_t) 0);
assert(nloc != (locale_t) 0);
uselocale(nloc);
bool useh = false;
int sz;
// How many characters required in the decimal string?
// Use a very large precision to get all the digits.
sz = std::snprintf(nullptr, 0, "%.100g", v);
// If the decimal size is 10 or less, use it.
if(sz > 10)
{
// How many characters required in the hex string?
const int hsz = std::snprintf(nullptr, 0, "%a", v);
// If the hex size is less than the decimal size, use it.
if(hsz < sz)
{
sz = hsz;
useh = true;
}
}
// Note +1 for null terminator since the returned size doesn't include it.
std::vector<char> buf(sz + 1);
if(useh)
std::snprintf(&buf[0], buf.size(), "%a", v);
else
std::snprintf(&buf[0], buf.size(), "%.100g", v);
// NOTE: LC_GLOBAL_HANDLE? Seems to be a mistake in the uselocale() documentation example.
//uselocale(LC_GLOBAL_HANDLE);
// So the new locale is no longer in use.
uselocale(LC_GLOBAL_LOCALE);
freelocale(nloc);
return QString(&buf[0]);
}
double museStringToDouble(const QString &s, bool *ok)
{
const char *sc = s.toLatin1().constData();
char *end;
// NOTE: strtod is locale sensitive! We must force the locale to standard 'C'.
// Use a per-thread thread-safe technique instead of setting the global locale.
// Note this C locale is NOT the same as the application's C++ locale.
// Setting LANG environment variable before running is different than setting our -l switch.
locale_t nloc = newlocale(LC_NUMERIC_MASK, "C", (locale_t) 0);
assert(nloc != (locale_t) 0);
uselocale(nloc);
const double rv = std::strtod(sc, &end);
if(ok)
// If the conversion fails, strtod sets end = input string.
*ok = end != sc;
// NOTE: LC_GLOBAL_HANDLE? Seems to be a mistake in the uselocale() documentation example.
//uselocale(LC_GLOBAL_HANDLE);
// So the new locale is no longer in use.
uselocale(LC_GLOBAL_LOCALE);
freelocale(nloc);
return rv;
}
//===========================
// Proposed new functions:
//===========================
QString museStringFromDouble_2(double v)
{
std::stringstream ss;
ss.precision(100);
ss.imbue(std::locale("C"));
ss << v;
if (ss.str().size() > 10)
{
ss.str("");
ss << std::hexfloat << v;
}
return QString::fromLatin1(ss.str().c_str());
}
double museStringToDouble_2(const QString &s, bool *ok)
{
return s.toDouble(ok);
}
//===========================
// Main function:
//===========================
int main(int argc, char *argv[])
{
const double d1 = 0.1234;
{
QString d1_string = museStringFromDouble(d1);
fprintf(stderr, "d1:%f d1_string:%s\n", d1, d1_string.toUtf8().constData());
bool ok = false;
const double d2 = museStringToDouble(d1_string, &ok);
fprintf(stderr, "d2:%f ok:%d\n", d2, ok);
}
{
QString d1_string_2 = museStringFromDouble_2(d1);
fprintf(stderr, "d1:%f d1_string_2:%s\n", d1, d1_string_2.toUtf8().constData());
bool ok = false;
const double d2_2 = museStringToDouble_2(d1_string_2, &ok);
fprintf(stderr, "d2_2:%f ok:%d\n", d2_2, ok);
}
return 0;
}
The above test produces the following output:
// Original functions:
d1:0.123400 d1_string:0x1.f972474538ef3p-4
d2:0.123400 ok:1
// Proposed new functions:
d1:0.123400 d1_string_2:0x1.f972474538ef3p-4
d2_2:0.000000 ok:0
I like the new museStringFromDouble(), it seems OK so far. But can you think of a way to make a better museStringToDouble()? It needs to be able to handle hex floats, as well as using only the c locale. (Only c locale is used because we avoid foreign number formats in song files, for maximum file portability.) I researched and found that Qt's own string functions do not seem to support hex floats at all. That is why museStringToDouble() was written that way. It may seem odd and out of place, but it works.
An important related note:
A few weeks ago, I switched our disk audio pre-fetch thread from real-time to normal priority. Real-time priority seemed unnecessary - hey, it's just a disk thread - so I lowered the priority. We were using linux pthreads to achieve real-time settings. And we are still using pthreads even though we don't need real-time any more.
So I made a note in the code and ChangeLog that this means we should be able to switch from using linux pthreads to using a simple QThread instead. That is an important step in porting to Mingw, because if I understand correctly, and as I warned you earlier, MusE cannot be built with Mingw if we are using linux pthreads. My understanding is that only CodeWeavers' Crossover products include pthreads support.
Note that we also use pthreads for our ALSA timer - however our ALSA support is disabled on Windows, and this means with the changes planned above, we could be completely free of any linux pthreads requirement when building for Windows :-)
I am still monitoring for feedback from users about whether the disk thread switch was a good move. So it may change back again if it becomes necessary. I hope it is not necessary.
donarturo11
Goals