From 859e95432e142a3d039635bcfe9f7a7d8e2f9619 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 12:23:19 +0200 Subject: [PATCH 01/21] [partition] Handle all enum values in the switch --- src/libcalamares/partition/PartitionSize.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libcalamares/partition/PartitionSize.cpp b/src/libcalamares/partition/PartitionSize.cpp index edff0fe1e..5c4c0e040 100644 --- a/src/libcalamares/partition/PartitionSize.cpp +++ b/src/libcalamares/partition/PartitionSize.cpp @@ -153,6 +153,9 @@ PartitionSize::toBytes() const switch ( m_unit ) { + case unit_t::None: + case unit_t::Percent: + return -1; case unit_t::Byte: return value(); case unit_t::KiB: @@ -161,12 +164,7 @@ PartitionSize::toBytes() const return CalamaresUtils::MiBtoBytes( static_cast( value() ) ); case unit_t::GiB: return CalamaresUtils::GiBtoBytes( static_cast( value() ) ); - default: - break; } - - // Reached only when unit is Percent or None - return -1; } bool From 7302b9c8510295128b2f291c64d3709eabdf3424 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 12:23:41 +0200 Subject: [PATCH 02/21] [libcalamares] Fix nested namespaces - Declaring namespace A::B is a C++17 extension, and Calamares is C++14. Split the namespace declarations. - While here, fix extra const warning as well. --- src/libcalamares/locale/Label.cpp | 5 ++++- src/libcalamares/locale/Label.h | 7 ++++--- src/libcalamares/locale/LabelModel.cpp | 7 +++++-- src/libcalamares/locale/LabelModel.h | 9 +++++---- src/libcalamares/locale/Lookup.cpp | 5 ++++- src/libcalamares/locale/Lookup.h | 6 ++++-- 6 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/libcalamares/locale/Label.cpp b/src/libcalamares/locale/Label.cpp index ca528dc75..8d3cd443f 100644 --- a/src/libcalamares/locale/Label.cpp +++ b/src/libcalamares/locale/Label.cpp @@ -19,7 +19,9 @@ #include "Label.h" -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { Label::Label() @@ -70,4 +72,5 @@ QLocale Label::getLocale( const QString& localeName ) return QLocale( localeName ); } +} } // namespace diff --git a/src/libcalamares/locale/Label.h b/src/libcalamares/locale/Label.h index 65befc6b4..7935b0880 100644 --- a/src/libcalamares/locale/Label.h +++ b/src/libcalamares/locale/Label.h @@ -23,8 +23,9 @@ #include #include -namespace CalamaresUtils {} -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { /** @@ -120,7 +121,7 @@ protected: QString m_englishLabel; } ; - +} } // namespace #endif diff --git a/src/libcalamares/locale/LabelModel.cpp b/src/libcalamares/locale/LabelModel.cpp index 543417212..ef8ff8ea1 100644 --- a/src/libcalamares/locale/LabelModel.cpp +++ b/src/libcalamares/locale/LabelModel.cpp @@ -22,7 +22,9 @@ #include "CalamaresVersion.h" // For the list of translations -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { LabelModel::LabelModel( const QStringList& locales, QObject* parent ) @@ -121,10 +123,11 @@ LabelModel::find( const QString& countryCode ) const return find( [&]( const Label& l ){ return l.language() == c_l.second; } ); } -LabelModel* const availableTranslations() +LabelModel* availableTranslations() { static LabelModel model( QString( CALAMARES_TRANSLATION_LANGUAGES ).split( ';') ); return &model; } +} } // namespace diff --git a/src/libcalamares/locale/LabelModel.h b/src/libcalamares/locale/LabelModel.h index 178f76343..2f3bee629 100644 --- a/src/libcalamares/locale/LabelModel.h +++ b/src/libcalamares/locale/LabelModel.h @@ -26,8 +26,9 @@ #include -namespace CalamaresUtils {} -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { class DLLEXPORT LabelModel : public QAbstractListModel @@ -78,7 +79,7 @@ private: * * NOTE: While the model is not typed const, it should be. Do not modify. */ -DLLEXPORT LabelModel* const availableTranslations(); - +DLLEXPORT LabelModel* availableTranslations(); +} } // namespace #endif diff --git a/src/libcalamares/locale/Lookup.cpp b/src/libcalamares/locale/Lookup.cpp index a096bc679..73d706de6 100644 --- a/src/libcalamares/locale/Lookup.cpp +++ b/src/libcalamares/locale/Lookup.cpp @@ -20,7 +20,9 @@ #include "CountryData_p.cpp" -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { struct TwoChar @@ -87,4 +89,5 @@ QLocale::Language languageForCountry(QLocale::Country country) return p->l; } +} } // namespace diff --git a/src/libcalamares/locale/Lookup.h b/src/libcalamares/locale/Lookup.h index 5712a1120..9d1c23cd8 100644 --- a/src/libcalamares/locale/Lookup.h +++ b/src/libcalamares/locale/Lookup.h @@ -24,8 +24,9 @@ #include #include -namespace CalamaresUtils {} -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { /* All the functions in this file do lookups of locale data * based on CLDR tables; these are lookups that you can't (easily) @@ -48,6 +49,7 @@ namespace CalamaresUtils::Locale DLLEXPORT QPair< QLocale::Country, QLocale::Language > countryData( const QString& code ); /// @brief Get a likely locale for a 2-letter country code DLLEXPORT QLocale countryLocale( const QString& code ); +} } // namespace #endif From 2c94cbdb14cf63e4d7ab593dce34de12d6dba608 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 12:25:14 +0200 Subject: [PATCH 03/21] [libcalamares] namespace A::B is a C++17 extension --- src/libcalamares/geoip/GeoIPJSON.cpp | 7 ++++--- src/libcalamares/geoip/GeoIPJSON.h | 5 ++++- src/libcalamares/geoip/GeoIPXML.h | 5 ++++- src/libcalamares/geoip/Handler.cpp | 6 ++++-- src/libcalamares/geoip/Handler.h | 6 ++++-- src/libcalamares/geoip/Interface.cpp | 5 ++++- src/libcalamares/geoip/Interface.h | 6 ++++-- 7 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/libcalamares/geoip/GeoIPJSON.cpp b/src/libcalamares/geoip/GeoIPJSON.cpp index 61b9fd8d6..4b7562d7e 100644 --- a/src/libcalamares/geoip/GeoIPJSON.cpp +++ b/src/libcalamares/geoip/GeoIPJSON.cpp @@ -25,7 +25,9 @@ #include -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { GeoIPJSON::GeoIPJSON(const QString& attribute) @@ -88,6 +90,5 @@ GeoIPJSON::processReply( const QByteArray& data ) return splitTZString( rawReply( data ) ); } - - +} } // namespace diff --git a/src/libcalamares/geoip/GeoIPJSON.h b/src/libcalamares/geoip/GeoIPJSON.h index 584825d70..4d7ded631 100644 --- a/src/libcalamares/geoip/GeoIPJSON.h +++ b/src/libcalamares/geoip/GeoIPJSON.h @@ -21,7 +21,9 @@ #include "Interface.h" -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { /** @brief GeoIP lookup for services that return JSON. * @@ -46,5 +48,6 @@ public: virtual QString rawReply(const QByteArray & ) override; } ; +} } // namespace #endif diff --git a/src/libcalamares/geoip/GeoIPXML.h b/src/libcalamares/geoip/GeoIPXML.h index 7dee2ecbe..356e88b12 100644 --- a/src/libcalamares/geoip/GeoIPXML.h +++ b/src/libcalamares/geoip/GeoIPXML.h @@ -21,7 +21,9 @@ #include "Interface.h" -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { /** @brief GeoIP lookup with XML data * @@ -46,5 +48,6 @@ public: virtual QString rawReply(const QByteArray & ) override; } ; +} } // namespace #endif diff --git a/src/libcalamares/geoip/Handler.cpp b/src/libcalamares/geoip/Handler.cpp index 1e8b03b26..4add3eb13 100644 --- a/src/libcalamares/geoip/Handler.cpp +++ b/src/libcalamares/geoip/Handler.cpp @@ -47,7 +47,9 @@ handlerTypes() return names; } -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { Handler::Handler() @@ -179,5 +181,5 @@ Handler::queryRaw() const } ); } - +} } // namespace diff --git a/src/libcalamares/geoip/Handler.h b/src/libcalamares/geoip/Handler.h index 92e5f326e..5c3207b60 100644 --- a/src/libcalamares/geoip/Handler.h +++ b/src/libcalamares/geoip/Handler.h @@ -25,8 +25,9 @@ #include #include -namespace CalamaresUtils {} -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { /** @brief Handle one complete GeoIP lookup. @@ -86,6 +87,7 @@ private: const QString m_selector; }; +} } // namespace #endif diff --git a/src/libcalamares/geoip/Interface.cpp b/src/libcalamares/geoip/Interface.cpp index 50aa04683..36e680aab 100644 --- a/src/libcalamares/geoip/Interface.cpp +++ b/src/libcalamares/geoip/Interface.cpp @@ -20,7 +20,9 @@ #include "utils/Logger.h" -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { Interface::Interface(const QString& e) @@ -51,4 +53,5 @@ splitTZString( const QString& tz ) return RegionZonePair( QString(), QString() ); } +} } // namespace diff --git a/src/libcalamares/geoip/Interface.h b/src/libcalamares/geoip/Interface.h index 4b2ff3a5a..7db8c4c91 100644 --- a/src/libcalamares/geoip/Interface.h +++ b/src/libcalamares/geoip/Interface.h @@ -27,8 +27,9 @@ class QByteArray; -namespace CalamaresUtils {} -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { /** @brief A Region, Zone pair of strings * @@ -94,5 +95,6 @@ protected: QString m_element; // string for selecting from data } ; +} } // namespace #endif From 0b0fb93e75244d1e41a8555c45488596e6fc11a1 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 12:33:03 +0200 Subject: [PATCH 04/21] [libcalamares] Remove redundant default: in case - the switch handles all values of the enum and the compiler should be smart enough to know that (therefore default isn't needed, nor the return afterwards). --- src/libcalamares/geoip/Handler.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libcalamares/geoip/Handler.cpp b/src/libcalamares/geoip/Handler.cpp index 4add3eb13..60404c2d6 100644 --- a/src/libcalamares/geoip/Handler.cpp +++ b/src/libcalamares/geoip/Handler.cpp @@ -112,8 +112,6 @@ create_interface( Handler::Type t, const QString& selector ) #else return nullptr; #endif - default: // there are no others - return nullptr; } } From d048975f15ff89d09e8c8b732e92213ce728a4fe Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:06:28 +0200 Subject: [PATCH 05/21] [libcalamares] One more nested namespace --- src/libcalamares/geoip/GeoIPXML.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/geoip/GeoIPXML.cpp b/src/libcalamares/geoip/GeoIPXML.cpp index a4b9bb146..b658042bf 100644 --- a/src/libcalamares/geoip/GeoIPXML.cpp +++ b/src/libcalamares/geoip/GeoIPXML.cpp @@ -23,7 +23,9 @@ #include #include -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { GeoIPXML::GeoIPXML( const QString& element ) @@ -87,4 +89,5 @@ GeoIPXML::processReply( const QByteArray& data ) return RegionZonePair(); } +} } // namespace From 93a68c3d5f140805d993e06c019b46da260b4588 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:32:14 +0200 Subject: [PATCH 06/21] [libcalamares] Add convenience method to check for unit-comparability - Not all kinds of units are comparable. Introduce a method in PartitionSize to check for comparability (this could also be a free method, but seems more tidy here because it is specifically about comparing in the context of partition sizes). --- src/libcalamares/partition/PartitionSize.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/libcalamares/partition/PartitionSize.h b/src/libcalamares/partition/PartitionSize.h index 13ffa5c70..975ebd887 100644 --- a/src/libcalamares/partition/PartitionSize.h +++ b/src/libcalamares/partition/PartitionSize.h @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2019, Collabora Ltd + * Copyright 2019, Adriaan de Groot * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -96,6 +97,20 @@ public: * @return the size in bytes, or -1 if it cannot be calculated. */ qint64 toBytes() const; + + /** @brief Are the units comparable? + * + * None units cannot be compared with anything. Percentages can + * be compared with each other, and all the explicit sizes (KiB, ...) + * can be compared with each other. + */ + static constexpr bool unitsComparable( const SizeUnit u1, const SizeUnit u2 ) + { + return !( ( u1 == SizeUnit::None || u2 == SizeUnit::None ) || + ( u1 == SizeUnit::Percent && u2 != SizeUnit::Percent ) || + ( u1 != SizeUnit::Percent && u2 == SizeUnit::Percent ) ); + } + }; } // namespace Calamares From 7a368dc1d748fbb77926a3f25ab47c51e4580015 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:33:38 +0200 Subject: [PATCH 07/21] [libcalamares] Add tests for the partitioning service --- src/libcalamares/CMakeLists.txt | 10 ++ src/libcalamares/partition/Tests.cpp | 145 +++++++++++++++++++++++++++ src/libcalamares/partition/Tests.h | 41 ++++++++ 3 files changed, 196 insertions(+) create mode 100644 src/libcalamares/partition/Tests.cpp create mode 100644 src/libcalamares/partition/Tests.h diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index 959b0a9db..b414887c4 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -167,6 +167,16 @@ if ( ECM_FOUND AND BUILD_TESTING ) ${YAMLCPP_LIBRARY} ) calamares_automoc( geoiptest ) + + ecm_add_test( + partition/Tests.cpp + TEST_NAME + libcalamarespartitiontest + LINK_LIBRARIES + calamares + Qt5::Test + ) + calamares_automoc( libcalamarespartitiontest ) endif() if( BUILD_TESTING ) diff --git a/src/libcalamares/partition/Tests.cpp b/src/libcalamares/partition/Tests.cpp new file mode 100644 index 000000000..ac2d4f431 --- /dev/null +++ b/src/libcalamares/partition/Tests.cpp @@ -0,0 +1,145 @@ +/* === This file is part of Calamares - === + * + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#include "Tests.h" + +#include "PartitionSize.h" + +Q_DECLARE_METATYPE( Calamares::SizeUnit ) + +#include "utils/Logger.h" + +#include + +QTEST_GUILESS_MAIN( PartitionSizeTests ) + +PartitionSizeTests::PartitionSizeTests() +{ +} + +PartitionSizeTests::~PartitionSizeTests() +{ +} + +void +PartitionSizeTests::initTestCase() +{ +} + +void +PartitionSizeTests::testUnitComparison_data() +{ + QTest::addColumn("u1"); + QTest::addColumn("u2"); + QTest::addColumn("comparable"); + + using Calamares::SizeUnit; + + QTest::newRow("nones") << SizeUnit::None << SizeUnit::None << false; + QTest::newRow("none+%") << SizeUnit::None << SizeUnit::Percent<< false; + QTest::newRow("%+none") << SizeUnit::Percent << SizeUnit::None << false; + QTest::newRow("KiB+none") << SizeUnit::KiB << SizeUnit::None << false; + QTest::newRow("none+MiB") << SizeUnit::None << SizeUnit::MiB << false; + + QTest::newRow("KiB+KiB") << SizeUnit::KiB << SizeUnit::KiB << true; + QTest::newRow("KiB+MiB") << SizeUnit::KiB << SizeUnit::MiB << true; + QTest::newRow("KiB+GiB") << SizeUnit::KiB << SizeUnit::GiB << true; + QTest::newRow("MiB+MiB") << SizeUnit::MiB << SizeUnit::MiB << true; + QTest::newRow("MiB+GiB") << SizeUnit::MiB << SizeUnit::GiB << true; + QTest::newRow("GiB+GiB") << SizeUnit::GiB << SizeUnit::GiB << true; + + QTest::newRow("%+None") << SizeUnit::Percent << SizeUnit::None << false; + QTest::newRow("%+%") << SizeUnit::Percent << SizeUnit::Percent << true; + QTest::newRow("%+KiB") << SizeUnit::Percent << SizeUnit::KiB << false; +} + + +static bool +original_compare( Calamares::SizeUnit m_unit, Calamares::SizeUnit other_m_unit ) +{ + if ( ( m_unit == Calamares::SizeUnit::None || other_m_unit == Calamares::SizeUnit::None ) || + ( m_unit == Calamares::SizeUnit::Percent && other_m_unit != Calamares::SizeUnit::Percent ) || + ( m_unit != Calamares::SizeUnit::Percent && other_m_unit == Calamares::SizeUnit::Percent ) ) + return false; + return true; +} + +void +PartitionSizeTests::testUnitComparison() +{ + QFETCH( Calamares::SizeUnit, u1 ); + QFETCH( Calamares::SizeUnit, u2 ); + QFETCH( bool, comparable ); + + if ( comparable ) + { + QVERIFY( Calamares::PartitionSize::unitsComparable( u1, u2 ) ); + QVERIFY( Calamares::PartitionSize::unitsComparable( u2, u1 ) ); + } + else + { + QVERIFY( !Calamares::PartitionSize::unitsComparable( u1, u2 ) ); + QVERIFY( !Calamares::PartitionSize::unitsComparable( u2, u1 ) ); + } + + QCOMPARE( original_compare( u1, u2 ), Calamares::PartitionSize::unitsComparable( u1, u2 ) ); +} + +void +PartitionSizeTests::testUnitNormalisation_data() +{ + QTest::addColumn("u1"); + QTest::addColumn("v"); + QTest::addColumn("bytes"); + + using Calamares::SizeUnit; + + QTest::newRow("none") << SizeUnit::None << 16 << -1; + QTest::newRow("none") << SizeUnit::None << 0 << -1; + QTest::newRow("none") << SizeUnit::None << -2 << -1; + + QTest::newRow("percent") << SizeUnit::Percent << 0 << -1; + QTest::newRow("percent") << SizeUnit::Percent << 16 << -1; + QTest::newRow("percent") << SizeUnit::Percent << -2 << -1; + + QTest::newRow("KiB") << SizeUnit::KiB << 0 << 0; + QTest::newRow("KiB") << SizeUnit::KiB << 1 << 1024; + QTest::newRow("KiB") << SizeUnit::KiB << 1000 << 1024000; + QTest::newRow("KiB") << SizeUnit::KiB << 1024 << 1024 * 1024; + QTest::newRow("KiB") << SizeUnit::KiB << -2 << -1; + + QTest::newRow("MiB") << SizeUnit::MiB << 0 << 0; + QTest::newRow("MiB") << SizeUnit::MiB << 1 << 1024 * 1024; + QTest::newRow("MiB") << SizeUnit::MiB << 1000 << 1024 * 1024000; + QTest::newRow("MiB") << SizeUnit::MiB << 1024 << 1024 * 1024 * 1024; + QTest::newRow("MiB") << SizeUnit::MiB << -2 << -1; + + QTest::newRow("GiB") << SizeUnit::GiB << 0 << 0; + QTest::newRow("GiB") << SizeUnit::GiB << 1 << 1024 * 1024 * 1024; + QTest::newRow("GiB") << SizeUnit::GiB << 2 << 2 * 1024 * 1024 * 1024; +} + +void +PartitionSizeTests::testUnitNormalisation() +{ + QFETCH( Calamares::SizeUnit, u1 ); + QFETCH( int, v ); + QFETCH( int, bytes ); + + QCOMPARE( Calamares::PartitionSize( v, u1 ).toBytes(), static_cast( bytes ) ); +} diff --git a/src/libcalamares/partition/Tests.h b/src/libcalamares/partition/Tests.h new file mode 100644 index 000000000..24398233d --- /dev/null +++ b/src/libcalamares/partition/Tests.h @@ -0,0 +1,41 @@ +/* === This file is part of Calamares - === + * + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#ifndef LIBCALAMARES_PARTITION_TESTS_H +#define LIBCALAMARES_PARTITION_TESTS_H + +#include + +class PartitionSizeTests : public QObject +{ + Q_OBJECT +public: + PartitionSizeTests(); + ~PartitionSizeTests() override; + +private Q_SLOTS: + void initTestCase(); + + void testUnitComparison_data(); + void testUnitComparison(); + + void testUnitNormalisation_data(); + void testUnitNormalisation(); +}; + +#endif From 72e1a36752605df35acecde6a01e437a167e8391 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:38:30 +0200 Subject: [PATCH 08/21] [libcalamares] Update partition service tests - Use long so that 2GiB fits in the values - Document special case of 0[KMG]iB --- src/libcalamares/partition/PartitionSize.h | 3 +- src/libcalamares/partition/Tests.cpp | 46 +++++++++++----------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/libcalamares/partition/PartitionSize.h b/src/libcalamares/partition/PartitionSize.h index 975ebd887..da15a5e80 100644 --- a/src/libcalamares/partition/PartitionSize.h +++ b/src/libcalamares/partition/PartitionSize.h @@ -92,7 +92,8 @@ public: /** @brief Convert the size to bytes. * * This method is only valid for sizes in Bytes, KiB, MiB or GiB. - * It will return -1 in any other case. + * It will return -1 in any other case. Note that 0KiB and 0MiB and + * 0GiB are considered **invalid** sizes and return -1. * * @return the size in bytes, or -1 if it cannot be calculated. */ diff --git a/src/libcalamares/partition/Tests.cpp b/src/libcalamares/partition/Tests.cpp index ac2d4f431..78d8a42a0 100644 --- a/src/libcalamares/partition/Tests.cpp +++ b/src/libcalamares/partition/Tests.cpp @@ -105,33 +105,33 @@ PartitionSizeTests::testUnitNormalisation_data() { QTest::addColumn("u1"); QTest::addColumn("v"); - QTest::addColumn("bytes"); + QTest::addColumn("bytes"); using Calamares::SizeUnit; - QTest::newRow("none") << SizeUnit::None << 16 << -1; - QTest::newRow("none") << SizeUnit::None << 0 << -1; - QTest::newRow("none") << SizeUnit::None << -2 << -1; + QTest::newRow("none") << SizeUnit::None << 16 << -1L; + QTest::newRow("none") << SizeUnit::None << 0 << -1L; + QTest::newRow("none") << SizeUnit::None << -2 << -1L; - QTest::newRow("percent") << SizeUnit::Percent << 0 << -1; - QTest::newRow("percent") << SizeUnit::Percent << 16 << -1; - QTest::newRow("percent") << SizeUnit::Percent << -2 << -1; + QTest::newRow("percent") << SizeUnit::Percent << 0 << -1L; + QTest::newRow("percent") << SizeUnit::Percent << 16 << -1L; + QTest::newRow("percent") << SizeUnit::Percent << -2 << -1L; - QTest::newRow("KiB") << SizeUnit::KiB << 0 << 0; - QTest::newRow("KiB") << SizeUnit::KiB << 1 << 1024; - QTest::newRow("KiB") << SizeUnit::KiB << 1000 << 1024000; - QTest::newRow("KiB") << SizeUnit::KiB << 1024 << 1024 * 1024; - QTest::newRow("KiB") << SizeUnit::KiB << -2 << -1; - - QTest::newRow("MiB") << SizeUnit::MiB << 0 << 0; - QTest::newRow("MiB") << SizeUnit::MiB << 1 << 1024 * 1024; - QTest::newRow("MiB") << SizeUnit::MiB << 1000 << 1024 * 1024000; - QTest::newRow("MiB") << SizeUnit::MiB << 1024 << 1024 * 1024 * 1024; - QTest::newRow("MiB") << SizeUnit::MiB << -2 << -1; - - QTest::newRow("GiB") << SizeUnit::GiB << 0 << 0; - QTest::newRow("GiB") << SizeUnit::GiB << 1 << 1024 * 1024 * 1024; - QTest::newRow("GiB") << SizeUnit::GiB << 2 << 2 * 1024 * 1024 * 1024; + QTest::newRow("KiB") << SizeUnit::KiB << 0 << -1L; + QTest::newRow("KiB") << SizeUnit::KiB << 1 << 1024L; + QTest::newRow("KiB") << SizeUnit::KiB << 1000 << 1024000L; + QTest::newRow("KiB") << SizeUnit::KiB << 1024 << 1024 * 1024L; + QTest::newRow("KiB") << SizeUnit::KiB << -2 << -1L; + + QTest::newRow("MiB") << SizeUnit::MiB << 0 << -1L; + QTest::newRow("MiB") << SizeUnit::MiB << 1 << 1024 * 1024L; + QTest::newRow("MiB") << SizeUnit::MiB << 1000 << 1024 * 1024000L; + QTest::newRow("MiB") << SizeUnit::MiB << 1024 << 1024 * 1024 * 1024L; + QTest::newRow("MiB") << SizeUnit::MiB << -2 << -1L; + + QTest::newRow("GiB") << SizeUnit::GiB << 0 << -1L; + QTest::newRow("GiB") << SizeUnit::GiB << 1 << 1024 * 1024 * 1024L; + QTest::newRow("GiB") << SizeUnit::GiB << 2 << 2048 * 1024 * 1024L; } void @@ -139,7 +139,7 @@ PartitionSizeTests::testUnitNormalisation() { QFETCH( Calamares::SizeUnit, u1 ); QFETCH( int, v ); - QFETCH( int, bytes ); + QFETCH( long, bytes ); QCOMPARE( Calamares::PartitionSize( v, u1 ).toBytes(), static_cast( bytes ) ); } From 90975b62bf167e2d08309b17935d1b3974a9916c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:44:30 +0200 Subject: [PATCH 09/21] [libcalamares] Tidy PartitionSize - Use unitsComparable where applicable - Use SizeUnit instead of unit_t -- since this is a template specialization, we have the more meaningful type name to use, instead of the generic one. --- src/libcalamares/partition/PartitionSize.cpp | 99 ++++++++++---------- src/libcalamares/partition/PartitionSize.h | 2 +- 2 files changed, 48 insertions(+), 53 deletions(-) diff --git a/src/libcalamares/partition/PartitionSize.cpp b/src/libcalamares/partition/PartitionSize.cpp index 5c4c0e040..98db92985 100644 --- a/src/libcalamares/partition/PartitionSize.cpp +++ b/src/libcalamares/partition/PartitionSize.cpp @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2019, Collabora Ltd + * Copyright 2019, Adriaan de Groot * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -42,23 +43,23 @@ unitSuffixes() PartitionSize::PartitionSize( const QString& s ) : NamedSuffix( unitSuffixes(), s ) { - if ( ( unit() == unit_t::Percent ) && ( value() > 100 || value() < 0 ) ) + if ( ( unit() == SizeUnit::Percent ) && ( value() > 100 || value() < 0 ) ) { cDebug() << "Percent value" << value() << "is not valid."; m_value = 0; } - if ( m_unit == unit_t::None ) + if ( m_unit == SizeUnit::None ) { m_value = s.toInt(); if ( m_value > 0 ) - m_unit = unit_t::Byte; + m_unit = SizeUnit::Byte; } if ( m_value <= 0 ) { m_value = 0; - m_unit = unit_t::None; + m_unit = SizeUnit::None; } } @@ -72,17 +73,17 @@ PartitionSize::toSectors( qint64 totalSectors, qint64 sectorSize ) const switch ( m_unit ) { - case unit_t::None: + case SizeUnit::None: return -1; - case unit_t::Percent: + case SizeUnit::Percent: if ( value() == 100 ) return totalSectors; // Common-case, avoid futzing around else return totalSectors * value() / 100; - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return CalamaresUtils::bytesToSectors ( toBytes(), sectorSize ); } @@ -97,19 +98,19 @@ PartitionSize::toBytes( qint64 totalSectors, qint64 sectorSize ) const switch ( m_unit ) { - case unit_t::None: + case SizeUnit::None: return -1; - case unit_t::Percent: + case SizeUnit::Percent: if ( totalSectors < 1 || sectorSize < 1 ) return -1; if ( value() == 100 ) return totalSectors * sectorSize; // Common-case, avoid futzing around else return totalSectors * value() / 100; - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return toBytes(); } @@ -125,19 +126,19 @@ PartitionSize::toBytes( qint64 totalBytes ) const switch ( m_unit ) { - case unit_t::None: + case SizeUnit::None: return -1; - case unit_t::Percent: + case SizeUnit::Percent: if ( totalBytes < 1 ) return -1; if ( value() == 100 ) return totalBytes; // Common-case, avoid futzing around else return totalBytes * value() / 100; - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return toBytes(); } @@ -153,16 +154,16 @@ PartitionSize::toBytes() const switch ( m_unit ) { - case unit_t::None: - case unit_t::Percent: + case SizeUnit::None: + case SizeUnit::Percent: return -1; - case unit_t::Byte: + case SizeUnit::Byte: return value(); - case unit_t::KiB: + case SizeUnit::KiB: return CalamaresUtils::KiBtoBytes( static_cast( value() ) ); - case unit_t::MiB: + case SizeUnit::MiB: return CalamaresUtils::MiBtoBytes( static_cast( value() ) ); - case unit_t::GiB: + case SizeUnit::GiB: return CalamaresUtils::GiBtoBytes( static_cast( value() ) ); } } @@ -170,19 +171,17 @@ PartitionSize::toBytes() const bool PartitionSize::operator< ( const PartitionSize& other ) const { - if ( ( m_unit == unit_t::None || other.m_unit == unit_t::None ) || - ( m_unit == unit_t::Percent && other.m_unit != unit_t::Percent ) || - ( m_unit != unit_t::Percent && other.m_unit == unit_t::Percent ) ) + if ( !unitsComparable( m_unit, other.m_unit ) ) return false; switch ( m_unit ) { - case unit_t::Percent: + case SizeUnit::Percent: return ( m_value < other.m_value ); - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return ( toBytes() < other.toBytes () ); } @@ -192,19 +191,17 @@ PartitionSize::operator< ( const PartitionSize& other ) const bool PartitionSize::operator> ( const PartitionSize& other ) const { - if ( ( m_unit == unit_t::None || other.m_unit == unit_t::None ) || - ( m_unit == unit_t::Percent && other.m_unit != unit_t::Percent ) || - ( m_unit != unit_t::Percent && other.m_unit == unit_t::Percent ) ) + if ( !unitsComparable( m_unit, other.m_unit ) ) return false; switch ( m_unit ) { - case unit_t::Percent: + case SizeUnit::Percent: return ( m_value > other.m_value ); - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return ( toBytes() > other.toBytes () ); } @@ -214,19 +211,17 @@ PartitionSize::operator> ( const PartitionSize& other ) const bool PartitionSize::operator== ( const PartitionSize& other ) const { - if ( ( m_unit == unit_t::None || other.m_unit == unit_t::None ) || - ( m_unit == unit_t::Percent && other.m_unit != unit_t::Percent ) || - ( m_unit != unit_t::Percent && other.m_unit == unit_t::Percent ) ) + if ( !unitsComparable( m_unit, other.m_unit ) ) return false; switch ( m_unit ) { - case unit_t::Percent: + case SizeUnit::Percent: return ( m_value == other.m_value ); - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return ( toBytes() == other.toBytes () ); } diff --git a/src/libcalamares/partition/PartitionSize.h b/src/libcalamares/partition/PartitionSize.h index da15a5e80..75ee960d9 100644 --- a/src/libcalamares/partition/PartitionSize.h +++ b/src/libcalamares/partition/PartitionSize.h @@ -49,7 +49,7 @@ class PartitionSize : public NamedSuffix { public: PartitionSize() : NamedSuffix() { } - PartitionSize( int v, unit_t u ) : NamedSuffix( v, u ) { } + PartitionSize( int v, SizeUnit u ) : NamedSuffix( v, u ) { } PartitionSize( const QString& ); bool isValid() const From 6db09f06796cf9abaa460b5c76ceabb1cee8fa42 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:54:09 +0200 Subject: [PATCH 10/21] [libcalamares] Handle all SizeUnit cases inside switch - Although None will be filtered out already by unitsComparable(), include it in the switch to avoid a warning .. then we can drop the post-switch return since the switch covers all possible values of the enum. --- src/libcalamares/partition/PartitionSize.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libcalamares/partition/PartitionSize.cpp b/src/libcalamares/partition/PartitionSize.cpp index 98db92985..8eb399585 100644 --- a/src/libcalamares/partition/PartitionSize.cpp +++ b/src/libcalamares/partition/PartitionSize.cpp @@ -176,6 +176,8 @@ PartitionSize::operator< ( const PartitionSize& other ) const switch ( m_unit ) { + case SizeUnit::None: + return false; case SizeUnit::Percent: return ( m_value < other.m_value ); case SizeUnit::Byte: @@ -184,8 +186,6 @@ PartitionSize::operator< ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() < other.toBytes () ); } - - return false; } bool @@ -196,6 +196,8 @@ PartitionSize::operator> ( const PartitionSize& other ) const switch ( m_unit ) { + case SizeUnit::None: + return false; case SizeUnit::Percent: return ( m_value > other.m_value ); case SizeUnit::Byte: @@ -204,8 +206,6 @@ PartitionSize::operator> ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() > other.toBytes () ); } - - return false; } bool @@ -216,6 +216,8 @@ PartitionSize::operator== ( const PartitionSize& other ) const switch ( m_unit ) { + case SizeUnit::None: + return false; case SizeUnit::Percent: return ( m_value == other.m_value ); case SizeUnit::Byte: @@ -224,8 +226,6 @@ PartitionSize::operator== ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() == other.toBytes () ); } - - return false; } } // namespace Calamares From ed3eafbc2db8c005d5f0adcc05e996b964fd69cd Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 14:08:30 +0200 Subject: [PATCH 11/21] [oemid] Reduce warnings about vtable by adding virtual destructor --- src/modules/oemid/OEMViewStep.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/modules/oemid/OEMViewStep.cpp b/src/modules/oemid/OEMViewStep.cpp index 0f076927b..8a12bd17c 100644 --- a/src/modules/oemid/OEMViewStep.cpp +++ b/src/modules/oemid/OEMViewStep.cpp @@ -43,9 +43,15 @@ public: ) } + virtual ~OEMPage() override; + Ui_OEMPage* m_ui; } ; +OEMPage::~OEMPage() +{ +} + OEMViewStep::OEMViewStep(QObject* parent) : Calamares::ViewStep( parent ) From ba7ee445c61b9833c318b7cdeaada88bf8ae3e82 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 14:52:26 +0200 Subject: [PATCH 12/21] CMake: switch to using autouic on plugins - Use autouic so that we can also pass in --include to add a code-warning-suppression to the generated code, just like we can do with moc. --- CMakeLists.txt | 1 + CMakeModules/CalamaresAddLibrary.cmake | 10 ++++------ CMakeModules/CalamaresAutomoc.cmake | 13 ++++++++++++- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 050191c1f..843e8bc69 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -211,6 +211,7 @@ if( CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) set( CMAKE_SHARED_LINKER_FLAGS "-Wl,--no-undefined" ) set( CALAMARES_AUTOMOC_OPTIONS "-butils/moc-warnings.h" ) + set( CALAMARES_AUTOUIC_OPTIONS --include utils/moc-warnings.h ) else() set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wl,--no-undefined" ) set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wl,--fatal-warnings -Wnon-virtual-dtor -Woverloaded-virtual -Werror=return-type" ) diff --git a/CMakeModules/CalamaresAddLibrary.cmake b/CMakeModules/CalamaresAddLibrary.cmake index e731e2b15..0829d919e 100644 --- a/CMakeModules/CalamaresAddLibrary.cmake +++ b/CMakeModules/CalamaresAddLibrary.cmake @@ -61,11 +61,6 @@ function(calamares_add_library) include_directories(${CMAKE_CURRENT_LIST_DIR}) include_directories(${CMAKE_CURRENT_BINARY_DIR}) - if(LIBRARY_UI) - qt5_wrap_ui(LIBRARY_UI_SOURCES ${LIBRARY_UI}) - list(APPEND LIBRARY_SOURCES ${LIBRARY_UI_SOURCES}) - endif() - # add resources from current dir if(EXISTS "${CMAKE_CURRENT_LIST_DIR}/${LIBRARY_RESOURCES}") qt5_add_resources(LIBRARY_RC_SOURCES "${LIBRARY_RESOURCES}") @@ -83,7 +78,10 @@ function(calamares_add_library) endif() calamares_automoc(${target}) - + if(LIBRARY_UI) + calamares_autouic(${target} ${LIBRARY_UI}) + endif() + if(LIBRARY_EXPORT_MACRO) set_target_properties(${target} PROPERTIES COMPILE_DEFINITIONS ${LIBRARY_EXPORT_MACRO}) endif() diff --git a/CMakeModules/CalamaresAutomoc.cmake b/CMakeModules/CalamaresAutomoc.cmake index 0ca5cd89a..f8aa7faef 100644 --- a/CMakeModules/CalamaresAutomoc.cmake +++ b/CMakeModules/CalamaresAutomoc.cmake @@ -18,7 +18,7 @@ # ### # -# Helper function for doing automoc on a target. +# Helper function for doing automoc on a target, and autoui on a .ui file. # # Sets AUTOMOC TRUE for a target. # @@ -27,6 +27,8 @@ # libcalamares/utils/moc-warnings.h file to the moc, which in turn # reduces compiler warnings in generated MOC code. # +# If the global variable CALAMARES_AUTOUIC_OPTIONS is set, adds that +# to the options passed to uic. function(calamares_automoc TARGET) set_target_properties( ${TARGET} PROPERTIES AUTOMOC TRUE ) @@ -34,3 +36,12 @@ function(calamares_automoc TARGET) set_target_properties( ${TARGET} PROPERTIES AUTOMOC_MOC_OPTIONS "${CALAMARES_AUTOMOC_OPTIONS}" ) endif() endfunction() + +function(calamares_autouic TARGET) + set_target_properties( ${TARGET} PROPERTIES AUTOUIC TRUE ) + if ( CALAMARES_AUTOUIC_OPTIONS ) + foreach(S ${ARGN}) + set_property(SOURCE ${S} PROPERTY AUTOUIC_OPTIONS "${CALAMARES_AUTOUIC_OPTIONS}") + endforeach() + endif() +endfunction() From c44eaf107faf7e44f8b663cea95f66583ea065af Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 04:39:56 -0400 Subject: [PATCH 13/21] CI: When stopping the build early, log where it was left --- ci/RELEASE.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ci/RELEASE.sh b/ci/RELEASE.sh index a835ebcb3..dfb65cfeb 100644 --- a/ci/RELEASE.sh +++ b/ci/RELEASE.sh @@ -41,8 +41,8 @@ BUILDDIR=$(mktemp -d --suffix=-build --tmpdir=.) if test "x$BUILD_DEFAULT" = "xtrue" ; then rm -rf "$BUILDDIR" mkdir "$BUILDDIR" || { echo "Could not create build directory." ; exit 1 ; } - ( cd "$BUILDDIR" && cmake .. && make -j4 ) || { echo "Could not perform test-build." ; exit 1 ; } - ( cd "$BUILDDIR" && make test ) || { echo "Tests failed." ; exit 1 ; } + ( cd "$BUILDDIR" && cmake .. && make -j4 ) || { echo "Could not perform test-build in $BUILDDIR." ; exit 1 ; } + ( cd "$BUILDDIR" && make test ) || { echo "Tests failed in $BUILDDIR." ; exit 1 ; } fi ### Build with clang @@ -53,13 +53,13 @@ if test "x$BUILD_CLANG" = "xtrue" ; then # Do build again with clang rm -rf "$BUILDDIR" mkdir "$BUILDDIR" || { echo "Could not create build directory." ; exit 1 ; } - ( cd "$BUILDDIR" && CC=clang CXX=clang++ cmake .. && make -j4 ) || { echo "Could not perform test-build." ; exit 1 ; } - ( cd "$BUILDDIR" && make test ) || { echo "Tests failed." ; exit 1 ; } + ( cd "$BUILDDIR" && CC=clang CXX=clang++ cmake .. && make -j4 ) || { echo "Could not perform test-build in $BUILDDIR." ; exit 1 ; } + ( cd "$BUILDDIR" && make test ) || { echo "Tests failed in $BUILDDIR." ; exit 1 ; } fi fi if test "x$BUILD_ONLY" = "xtrue" ; then - echo "Builds completed, release stopped." + echo "Builds completed, release stopped. Build remains in $BUILDDIR." exit 1 fi From 10ba46874825b9d69bb8e856676bd1a4afc8062d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 12:04:31 -0400 Subject: [PATCH 14/21] [libcalamares] Avoid warnings / errors on both gcc and clang - Clang 8 can detect that there is no need for a return if all previous paths already return. GCC 8 does not. Clang warns if the unreachable return is there, GCC errors out if it isn't. - Introduce a hack NOTREACHED that comments-out on Clang, and marks as unreachable (but still present) on GCC. - This might go away with an [[unreachable]] annotation or similar. --- CMakeLists.txt | 3 +++ src/libcalamares/geoip/Handler.cpp | 1 + src/libcalamares/partition/PartitionSize.cpp | 4 ++++ 3 files changed, 8 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 843e8bc69..a4571efba 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -193,6 +193,7 @@ if( CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) ) string( APPEND CMAKE_CXX_FLAGS " ${CLANG_WARNINGS}" ) endforeach() + set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNOTREACHED='//'" ) # Third-party code where we don't care so much about compiler warnings # (because it's uncomfortable to patch) get different flags; use @@ -218,6 +219,8 @@ else() set( SUPPRESS_3RDPARTY_WARNINGS "" ) set( SUPPRESS_BOOST_WARNINGS "" ) + + set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNOTREACHED='__builtin_unreachable();'" ) endif() # Use mark_thirdparty_code() to reduce warnings from the compiler diff --git a/src/libcalamares/geoip/Handler.cpp b/src/libcalamares/geoip/Handler.cpp index 60404c2d6..192ab1a7c 100644 --- a/src/libcalamares/geoip/Handler.cpp +++ b/src/libcalamares/geoip/Handler.cpp @@ -113,6 +113,7 @@ create_interface( Handler::Type t, const QString& selector ) return nullptr; #endif } + NOTREACHED return nullptr; } static RegionZonePair diff --git a/src/libcalamares/partition/PartitionSize.cpp b/src/libcalamares/partition/PartitionSize.cpp index 8eb399585..750f75393 100644 --- a/src/libcalamares/partition/PartitionSize.cpp +++ b/src/libcalamares/partition/PartitionSize.cpp @@ -166,6 +166,7 @@ PartitionSize::toBytes() const case SizeUnit::GiB: return CalamaresUtils::GiBtoBytes( static_cast( value() ) ); } + NOTREACHED return -1; } bool @@ -186,6 +187,7 @@ PartitionSize::operator< ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() < other.toBytes () ); } + NOTREACHED return false; } bool @@ -206,6 +208,7 @@ PartitionSize::operator> ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() > other.toBytes () ); } + NOTREACHED return false; } bool @@ -226,6 +229,7 @@ PartitionSize::operator== ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() == other.toBytes () ); } + NOTREACHED return false; } } // namespace Calamares From bffaf47900ace8ce1a3ebe80966f74010a5b216b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 04:39:19 -0400 Subject: [PATCH 15/21] [partition] Reduce warnings about integer size --- src/modules/partition/gui/ResizeVolumeGroupDialog.cpp | 2 +- src/modules/partition/gui/VolumeGroupBaseDialog.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/partition/gui/ResizeVolumeGroupDialog.cpp b/src/modules/partition/gui/ResizeVolumeGroupDialog.cpp index 1c5eef0ab..2de999360 100644 --- a/src/modules/partition/gui/ResizeVolumeGroupDialog.cpp +++ b/src/modules/partition/gui/ResizeVolumeGroupDialog.cpp @@ -44,7 +44,7 @@ ResizeVolumeGroupDialog::ResizeVolumeGroupDialog( LvmDevice *device, for ( const Partition* p : availablePVs ) pvList()->addItem( new ListPhysicalVolumeWidgetItem( p, false ) ); - peSize()->setValue( device->peSize() / Capacity::unitFactor(Capacity::Unit::Byte, Capacity::Unit::MiB) ); + peSize()->setValue( static_cast( device->peSize() / Capacity::unitFactor(Capacity::Unit::Byte, Capacity::Unit::MiB) ) ); vgName()->setEnabled( false ); peSize()->setEnabled( false ); diff --git a/src/modules/partition/gui/VolumeGroupBaseDialog.cpp b/src/modules/partition/gui/VolumeGroupBaseDialog.cpp index a727fe42a..8078253b3 100644 --- a/src/modules/partition/gui/VolumeGroupBaseDialog.cpp +++ b/src/modules/partition/gui/VolumeGroupBaseDialog.cpp @@ -137,9 +137,9 @@ VolumeGroupBaseDialog::updateTotalSize() void VolumeGroupBaseDialog::updateTotalSectors() { - qint32 totalSectors = 0; + qint64 totalSectors = 0; - qint32 extentSize = ui->peSize->value() * Capacity::unitFactor(Capacity::Unit::Byte, Capacity::Unit::MiB); + qint64 extentSize = ui->peSize->value() * Capacity::unitFactor(Capacity::Unit::Byte, Capacity::Unit::MiB); if ( extentSize > 0 ) totalSectors = m_totalSizeValue / extentSize; From fd4bc4bb17b2758db216204f7196c4d0356bc3ec Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 04:46:21 -0400 Subject: [PATCH 16/21] [partition] Avoid UB by initializing size everywhere --- src/modules/partition/core/PartitionLayout.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/partition/core/PartitionLayout.cpp b/src/modules/partition/core/PartitionLayout.cpp index a988da3f7..de5535a3e 100644 --- a/src/modules/partition/core/PartitionLayout.cpp +++ b/src/modules/partition/core/PartitionLayout.cpp @@ -150,7 +150,7 @@ PartitionLayout::execute( Device *dev, qint64 firstSector, const PartitionRole& role ) { QList< Partition* > partList; - qint64 size, minSize, maxSize, end; + qint64 minSize, maxSize, end; qint64 totalSize = lastSector - firstSector + 1; qint64 availableSize = totalSize; @@ -161,6 +161,7 @@ PartitionLayout::execute( Device *dev, qint64 firstSector, { Partition *currentPartition = nullptr; + qint64 size = -1; // Calculate partition size if ( part.partSize.isValid() ) { From 54108d2bab337dd42b10eea93658a675beae907d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 04:49:53 -0400 Subject: [PATCH 17/21] [partition] Fix up logging of jobs - Logging `*it` was printing raw pointers, logging (plain) `it` needs the specialized logging `operator<<` to DTRT with temporaries. --- src/modules/partition/jobs/CreatePartitionTableJob.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/partition/jobs/CreatePartitionTableJob.cpp b/src/modules/partition/jobs/CreatePartitionTableJob.cpp index 937b8437d..3465a0e2d 100644 --- a/src/modules/partition/jobs/CreatePartitionTableJob.cpp +++ b/src/modules/partition/jobs/CreatePartitionTableJob.cpp @@ -68,7 +68,7 @@ CreatePartitionTableJob::prettyStatusMessage() const static inline QDebug& -operator <<( QDebug& s, PartitionIterator& it ) +operator <<( QDebug&& s, PartitionIterator& it ) { s << ( ( *it ) ? ( *it )->deviceNode() : QString( "" ) ); return s; @@ -89,7 +89,7 @@ CreatePartitionTableJob::exec() { for ( auto it = PartitionIterator::begin( table ); it != PartitionIterator::end( table ); ++it ) - cDebug() << *it; + cDebug() << it; QProcess lsblk; lsblk.setProgram( "lsblk" ); From 81715ba1999f382a0fd201806bdb5b28021d94b8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 04:58:19 -0400 Subject: [PATCH 18/21] [partition] Warnings-- by using nullptr instead of 0 --- src/modules/partition/gui/PartitionPage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index 790ff84ab..647a69d5d 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -564,7 +564,7 @@ PartitionPage::updateFromCurrentDevice() QAbstractItemModel* oldModel = m_ui->partitionTreeView->model(); if ( oldModel ) - disconnect( oldModel, 0, this, 0 ); + disconnect( oldModel, nullptr, this, nullptr ); PartitionModel* model = m_core->partitionModelForDevice( device ); m_ui->partitionBarsView->setModel( model ); From 7e12b65c94a04bfaa92331712c0886f8d825494c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 05:01:39 -0400 Subject: [PATCH 19/21] [partition] Silence warnings about missing vtable --- src/modules/partition/tests/PartitionJobTests.cpp | 5 +++++ src/modules/partition/tests/PartitionJobTests.h | 1 + 2 files changed, 6 insertions(+) diff --git a/src/modules/partition/tests/PartitionJobTests.cpp b/src/modules/partition/tests/PartitionJobTests.cpp index e4707accf..acb671254 100644 --- a/src/modules/partition/tests/PartitionJobTests.cpp +++ b/src/modules/partition/tests/PartitionJobTests.cpp @@ -130,6 +130,11 @@ QueueRunner::QueueRunner( JobQueue* queue ) connect( m_queue, &JobQueue::failed, this, &QueueRunner::onFailed ); } +QueueRunner::~QueueRunner() +{ + // Nothing to do. We don't own the queue, and disconnect happens automatically +} + bool QueueRunner::run() { diff --git a/src/modules/partition/tests/PartitionJobTests.h b/src/modules/partition/tests/PartitionJobTests.h index 0744cbdda..62d5924ea 100644 --- a/src/modules/partition/tests/PartitionJobTests.h +++ b/src/modules/partition/tests/PartitionJobTests.h @@ -36,6 +36,7 @@ class QueueRunner : public QObject { public: QueueRunner( Calamares::JobQueue* queue ); + virtual ~QueueRunner() override; /** * Synchronously runs the queue. Returns true on success From 80606cc38d3b67f8af70fda5a351f3cbe7c3a15c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 05:27:38 -0400 Subject: [PATCH 20/21] [partition] Reduce test warnings through consistent signedness --- .../partition/tests/PartitionJobTests.cpp | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/modules/partition/tests/PartitionJobTests.cpp b/src/modules/partition/tests/PartitionJobTests.cpp index acb671254..f3bd8dd13 100644 --- a/src/modules/partition/tests/PartitionJobTests.cpp +++ b/src/modules/partition/tests/PartitionJobTests.cpp @@ -77,7 +77,7 @@ static QByteArray generateTestData( qint64 size ) { QByteArray ba; - ba.resize( size ); + ba.resize( static_cast( size ) ); // Fill the array explicitly to keep Valgrind happy for ( auto it = ba.data() ; it < ba.data() + size ; ++it ) { @@ -172,7 +172,8 @@ PartitionJobTests::initTestCase() QString devicePath = qgetenv( "CALAMARES_TEST_DISK" ); if ( devicePath.isEmpty() ) { - QSKIP( "Skipping test, CALAMARES_TEST_DISK is not set. It should point to a disk which can be safely formatted" ); + // The 0 is to keep the macro parameters happy + QSKIP( "Skipping test, CALAMARES_TEST_DISK is not set. It should point to a disk which can be safely formatted", 0 ); } QVERIFY( KPMHelpers::initKPMcore() ); @@ -327,10 +328,10 @@ PartitionJobTests::testCreatePartitionExtended() void PartitionJobTests::testResizePartition_data() { - QTest::addColumn< int >( "oldStartMiB" ); - QTest::addColumn< int >( "oldSizeMiB" ); - QTest::addColumn< int >( "newStartMiB" ); - QTest::addColumn< int >( "newSizeMiB" ); + QTest::addColumn< unsigned int >( "oldStartMiB" ); + QTest::addColumn< unsigned int >( "oldSizeMiB" ); + QTest::addColumn< unsigned int >( "newStartMiB" ); + QTest::addColumn< unsigned int >( "newSizeMiB" ); QTest::newRow("grow") << 10 << 50 << 10 << 70; QTest::newRow("shrink") << 10 << 70 << 10 << 50; @@ -341,10 +342,10 @@ PartitionJobTests::testResizePartition_data() void PartitionJobTests::testResizePartition() { - QFETCH( int, oldStartMiB ); - QFETCH( int, oldSizeMiB ); - QFETCH( int, newStartMiB ); - QFETCH( int, newSizeMiB ); + QFETCH( unsigned int, oldStartMiB ); + QFETCH( unsigned int, oldSizeMiB ); + QFETCH( unsigned int, newStartMiB ); + QFETCH( unsigned int, newSizeMiB ); const qint64 sectorsPerMiB = 1_MiB / m_device->logicalSize(); From e520c66bb9c332872c5b1ccf14b67cab65a948e3 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 05:11:07 -0400 Subject: [PATCH 21/21] [fsresizer] Give the FSResizerJob some accessors - This is primarily for the tests: then they can drop the #define private public hack and be "proper" consumers. --- src/modules/fsresizer/ResizeFSJob.h | 15 ++++++++ src/modules/fsresizer/Tests.cpp | 53 +++++++++++++---------------- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/src/modules/fsresizer/ResizeFSJob.h b/src/modules/fsresizer/ResizeFSJob.h index 97696e40b..79828027f 100644 --- a/src/modules/fsresizer/ResizeFSJob.h +++ b/src/modules/fsresizer/ResizeFSJob.h @@ -54,6 +54,21 @@ public: m_size.isValid(); } + QString name() const + { + return m_fsname.isEmpty() ? m_devicename : m_fsname; + } + + Calamares::PartitionSize size() const + { + return m_size; + } + + Calamares::PartitionSize minimumSize() const + { + return m_atleast; + } + private: Calamares::PartitionSize m_size; Calamares::PartitionSize m_atleast; diff --git a/src/modules/fsresizer/Tests.cpp b/src/modules/fsresizer/Tests.cpp index 5e645a95f..ecb2fea9d 100644 --- a/src/modules/fsresizer/Tests.cpp +++ b/src/modules/fsresizer/Tests.cpp @@ -30,9 +30,7 @@ #include #include -#define private public #include "ResizeFSJob.h" -#undef private QTEST_GUILESS_MAIN( FSResizerTests ) @@ -55,10 +53,9 @@ void FSResizerTests::testConfigurationRobust() // Empty config j.setConfigurationMap( QVariantMap() ); - QVERIFY( j.m_fsname.isEmpty() ); - QVERIFY( j.m_devicename.isEmpty() ); - QCOMPARE( j.m_size.unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.m_atleast.unit(), Calamares::SizeUnit::None ); + QVERIFY( j.name().isEmpty() ); + QCOMPARE( j.size().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); // Config is missing fs and dev, so it isn't valid YAML::Node doc0 = YAML::Load( R"(--- @@ -66,12 +63,11 @@ size: 100% atleast: 600MiB )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); - QVERIFY( j.m_fsname.isEmpty() ); - QVERIFY( j.m_devicename.isEmpty() ); - QCOMPARE( j.m_size.unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.m_atleast.unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.m_size.value(), 0 ); - QCOMPARE( j.m_atleast.value(), 0 ); + QVERIFY( j.name().isEmpty() ); + QCOMPARE( j.size().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.size().value(), 0 ); + QCOMPARE( j.minimumSize().value(), 0 ); } void FSResizerTests::testConfigurationValues() @@ -85,12 +81,11 @@ size: 100% atleast: 600MiB )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); - QVERIFY( !j.m_fsname.isEmpty() ); - QVERIFY( j.m_devicename.isEmpty() ); - QCOMPARE( j.m_size.unit(), Calamares::SizeUnit::Percent ); - QCOMPARE( j.m_atleast.unit(), Calamares::SizeUnit::MiB ); - QCOMPARE( j.m_size.value(), 100 ); - QCOMPARE( j.m_atleast.value(), 600 ); + QVERIFY( j.name().isEmpty() ); + QCOMPARE( j.size().unit(), Calamares::SizeUnit::Percent ); + QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::MiB ); + QCOMPARE( j.size().value(), 100 ); + QCOMPARE( j.minimumSize().value(), 600 ); // Silly config doc0 = YAML::Load( R"(--- @@ -100,12 +95,11 @@ size: 72 MiB atleast: 127 % )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); - QVERIFY( !j.m_fsname.isEmpty() ); - QVERIFY( !j.m_devicename.isEmpty() ); - QCOMPARE( j.m_size.unit(), Calamares::SizeUnit::MiB ); - QCOMPARE( j.m_atleast.unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.m_size.value(), 72 ); - QCOMPARE( j.m_atleast.value(), 0 ); + QVERIFY( !j.name().isEmpty() ); + QCOMPARE( j.size().unit(), Calamares::SizeUnit::MiB ); + QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.size().value(), 72 ); + QCOMPARE( j.minimumSize().value(), 0 ); // Silly config doc0 = YAML::Load( R"(--- @@ -115,10 +109,9 @@ size: 71MiB # atleast: 127% )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); - QVERIFY( !j.m_fsname.isEmpty() ); - QVERIFY( j.m_devicename.isEmpty() ); - QCOMPARE( j.m_size.unit(), Calamares::SizeUnit::MiB ); - QCOMPARE( j.m_atleast.unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.m_size.value(), 71 ); - QCOMPARE( j.m_atleast.value(), 0 ); + QVERIFY( j.name().isEmpty() ); + QCOMPARE( j.size().unit(), Calamares::SizeUnit::MiB ); + QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.size().value(), 71 ); + QCOMPARE( j.minimumSize().value(), 0 ); }