1
0
mirror of https://github.com/qTox/qTox.git synced 2024-03-22 14:00:36 +08:00

refactor(Paths): Remove Paths factory to enable copy construction

std::atomic disallows copy construction and the default constructor disables
the parameterized constructor.

Additionally the case where paths aren't writable isn't handled and would just
segfault in Settings previously, so no safety is lost.
This commit is contained in:
Anthony Bilinski 2022-02-09 20:19:29 -08:00
parent 0eb56fb9bb
commit 557af80428
No known key found for this signature in database
GPG Key ID: 2AA8E0DA1B31FB3C
5 changed files with 56 additions and 61 deletions

View File

@ -29,6 +29,8 @@
#include <QStringList>
#include <QSettings>
#include <cassert>
namespace {
const QLatin1String globalSettingsFile{"qtox.ini"};
const QLatin1String profileFolder{"profiles"};
@ -74,56 +76,58 @@ const QLatin1String TCSToxFileFolder{"~/.config/tox/"};
* All qTox or Tox specific directories should be looked up through this module.
*/
/**
* @brief Paths::makePaths Factory method for the Paths object
* @param mode
* @return Pointer to Paths object on success, nullptr else
*/
Paths* Paths::makePaths(Portable mode)
{
bool portable = false;
const QString basePortable = qApp->applicationDirPath();
const QString baseNonPortable = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation);
const QString portableSettingsPath = basePortable % QDir::separator() % globalSettingsFile;
namespace {
using Portable = Paths::Portable;
bool portableFromMode(Portable mode)
{
bool portable = false;
const QString basePortable = qApp->applicationDirPath();
const QString baseNonPortable = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation);
const QString portableSettingsPath = basePortable % QDir::separator() % globalSettingsFile;
switch (mode) {
case Portable::Portable:
qDebug() << "Forcing portable";
portable = true;
break;
case Portable::NonPortable:
qDebug() << "Forcing non-portable";
portable = false;
break;
case Portable::Auto:
if (QFile(portableSettingsPath).exists()) {
QSettings ps(portableSettingsPath, QSettings::IniFormat);
ps.setIniCodec("UTF-8");
ps.beginGroup("Advanced");
portable = ps.value("makeToxPortable", false).toBool();
ps.endGroup();
} else {
portable = false;
switch (mode) {
case Portable::Portable:
qDebug() << "Forcing portable";
return true;
case Portable::NonPortable:
qDebug() << "Forcing non-portable";
return false;
case Portable::Auto:
if (QFile(portableSettingsPath).exists()) {
QSettings ps(portableSettingsPath, QSettings::IniFormat);
ps.setIniCodec("UTF-8");
ps.beginGroup("Advanced");
portable = ps.value("makeToxPortable", false).toBool();
ps.endGroup();
} else {
portable = false;
}
qDebug()<< "Auto portable mode:" << portable;
return portable;
}
qDebug()<< "Auto portable mode:" << portable;
break;
assert(!"Unhandled enum class Paths::Portable value");
return false;
}
QString basePath = portable ? basePortable : baseNonPortable;
if (basePath.isEmpty()) {
qCritical() << "Couldn't find writable path";
return nullptr;
QString basePathFromPortable(bool portable)
{
const QString basePortable = qApp->applicationDirPath();
const QString baseNonPortable = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation);
QString basePath = portable ? basePortable : baseNonPortable;
if (basePath.isEmpty()) {
assert(!"Couldn't find writeable path");
qCritical() << "Couldn't find writable path";
}
return basePath;
}
} // namespace
return new Paths(basePath, portable);
Paths::Paths(Portable mode)
{
portable = portableFromMode(mode);
basePath = basePathFromPortable(portable);
}
Paths::Paths(const QString& basePath, bool portable)
: basePath{basePath}
, portable{portable}
{}
/**
* @brief Set paths to passed portable or system wide
* @param newPortable

View File

@ -34,7 +34,7 @@ public:
NonPortable /** Force non-portable mode */
};
static Paths* makePaths(Portable mode = Portable::Auto);
Paths(Portable mode = Portable::Auto);
bool setPortable(bool portable);
bool isPortable() const;
@ -54,11 +54,7 @@ public:
QString getUserNodesFilePath() const;
#endif
private:
Paths(const QString &basePath, bool portable);
private:
QString basePath{};
QString basePath;
std::atomic_bool portable{false};
};

View File

@ -67,7 +67,6 @@ Settings::Settings()
, useCustomDhtList{false}
, makeToxPortable{false}
, currentProfileId(0)
, paths(*Paths::makePaths(Paths::Portable::Auto))
{
settingsThread = new QThread();
settingsThread->setObjectName("qTox Settings");
@ -873,7 +872,6 @@ void Settings::setMakeToxPortable(bool newValue)
bool changed = false;
{
QMutexLocker locker{&bigLock};
QFile(paths.getSettingsDirPath() + globalSettingsFile).remove()
if (newValue != makeToxPortable) {
QFile(paths.getSettingsDirPath() + globalSettingsFile).remove();
makeToxPortable = newValue;

View File

@ -48,9 +48,9 @@ TestBootstrapNodesUpdater::TestBootstrapNodesUpdater()
void TestBootstrapNodesUpdater::testOnline()
{
QNetworkProxy proxy{QNetworkProxy::ProxyType::NoProxy};
auto paths = Paths::makePaths(Paths::Portable::NonPortable);
Paths paths{Paths::Portable::NonPortable};
BootstrapNodeUpdater updater{proxy, *paths};
BootstrapNodeUpdater updater{proxy, paths};
QSignalSpy spy(&updater, &BootstrapNodeUpdater::availableBootstrapNodes);
updater.requestBootstrapNodes();

View File

@ -56,11 +56,10 @@ const QString sep{QDir::separator()};
*/
void TestPaths::constructAuto()
{
Paths * paths = Paths::makePaths(Paths::Portable::Auto);
Paths paths{Paths::Portable::Auto};
// auto detection should succeed
QVERIFY(paths != nullptr);
// the test environment should not provide a `qtox.ini`
QVERIFY(paths->isPortable() == false);
QVERIFY(paths.isPortable() == false);
}
/**
@ -68,10 +67,9 @@ void TestPaths::constructAuto()
*/
void TestPaths::constructPortable()
{
Paths * paths = Paths::makePaths(Paths::Portable::Portable);
Paths paths{Paths::Portable::Portable};
// portable construction should succeed even though qtox.ini doesn't exist
QVERIFY(paths != nullptr);
QVERIFY(paths->isPortable() == true);
QVERIFY(paths.isPortable() == true);
}
/**
@ -79,11 +77,10 @@ void TestPaths::constructPortable()
*/
void TestPaths::constructNonPortable()
{
Paths * paths = Paths::makePaths(Paths::Portable::NonPortable);
Paths paths{Paths::Portable::NonPortable};
// Non portable should succeed
QVERIFY(paths != nullptr);
// the test environment should not provide a `qtox.ini`
QVERIFY(paths->isPortable() == false);
QVERIFY(paths.isPortable() == false);
}
/**