From 4473d7f5dd4f45fb6f2af3f92befb49f666887d5 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 22 Jun 2020 16:18:10 -0400 Subject: [PATCH 01/13] [preservefiles] Move permissions classes to libcalamares --- .../permissions.cpp => libcalamares/utils/Permissions.cpp} | 0 .../permissions.h => libcalamares/utils/Permissions.h} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/{modules/preservefiles/permissions.cpp => libcalamares/utils/Permissions.cpp} (100%) rename src/{modules/preservefiles/permissions.h => libcalamares/utils/Permissions.h} (100%) diff --git a/src/modules/preservefiles/permissions.cpp b/src/libcalamares/utils/Permissions.cpp similarity index 100% rename from src/modules/preservefiles/permissions.cpp rename to src/libcalamares/utils/Permissions.cpp diff --git a/src/modules/preservefiles/permissions.h b/src/libcalamares/utils/Permissions.h similarity index 100% rename from src/modules/preservefiles/permissions.h rename to src/libcalamares/utils/Permissions.h From e24f812b2d29e2833668b2c4a553a9d9ef1e496b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 22 Jun 2020 16:32:47 -0400 Subject: [PATCH 02/13] [libcalamares] Chase Permissions move - Fix include names in *preservefiles* - Tidy up include guards - Fix CMakeLists in *perservefiles* and *libcalamares* - Use SPDX license headers --- src/libcalamares/CMakeLists.txt | 1 + src/libcalamares/utils/Permissions.cpp | 67 ++++++-------- src/libcalamares/utils/Permissions.h | 41 ++++----- src/modules/preservefiles/CMakeLists.txt | 1 - src/modules/preservefiles/PreserveFiles.cpp | 99 ++++++++++++--------- src/modules/preservefiles/PreserveFiles.h | 32 +++---- 6 files changed, 114 insertions(+), 127 deletions(-) diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index 0516b5613..56bacb32a 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -76,6 +76,7 @@ set( libSources utils/Dirs.cpp utils/Entropy.cpp utils/Logger.cpp + utils/Permissions.cpp utils/PluginFactory.cpp utils/Retranslator.cpp utils/String.cpp diff --git a/src/libcalamares/utils/Permissions.cpp b/src/libcalamares/utils/Permissions.cpp index a3f8ac136..d9d3226e6 100644 --- a/src/libcalamares/utils/Permissions.cpp +++ b/src/libcalamares/utils/Permissions.cpp @@ -1,75 +1,66 @@ /* === This file is part of Calamares - === * - * Copyright (C) 2018 Scott Harvey - * - * This program 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. - * - * This program 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 this program. If not, see . + * SPDX-FileCopyrightText: 2018 Scott Harvey + * SPDX-License-Identifier: GPL-3.0-or-later + * License-Filename: LICENSE * */ +#include "Permissions.h" + #include #include -#include "permissions.h" -Permissions::Permissions() : - m_username(), - m_group(), - m_valid(false), - m_value(0) +Permissions::Permissions() + : m_username() + , m_group() + , m_valid( false ) + , m_value( 0 ) { } -Permissions::Permissions(QString p) : Permissions() +Permissions::Permissions( QString p ) + : Permissions() { - parsePermissions(p); + parsePermissions( p ); } -void Permissions::parsePermissions(const QString& p) { +void +Permissions::parsePermissions( const QString& p ) +{ - QStringList segments = p.split(":"); + QStringList segments = p.split( ":" ); - if (segments.length() != 3) { + if ( segments.length() != 3 ) + { m_valid = false; return; } - if (segments[0].isEmpty() || segments[1].isEmpty()) { + if ( segments[ 0 ].isEmpty() || segments[ 1 ].isEmpty() ) + { m_valid = false; return; } bool ok; - int octal = segments[2].toInt(&ok, 8); - if (!ok || octal == 0) { + int octal = segments[ 2 ].toInt( &ok, 8 ); + if ( !ok || octal == 0 ) + { m_valid = false; return; - } else { + } + else + { m_value = octal; } // We have exactly three segments and the third is valid octal, // so we can declare the string valid and set the user and group names m_valid = true; - m_username = segments[0]; - m_group = segments[1]; + m_username = segments[ 0 ]; + m_group = segments[ 1 ]; return; - } - - - - - - diff --git a/src/libcalamares/utils/Permissions.h b/src/libcalamares/utils/Permissions.h index 4cb70a2c2..baa5da554 100644 --- a/src/libcalamares/utils/Permissions.h +++ b/src/libcalamares/utils/Permissions.h @@ -1,45 +1,35 @@ /* === This file is part of Calamares - === * - * Copyright (C) 2018 Scott Harvey - * - * This program 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. - * - * This program 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 this program. If not, see . + * SPDX-FileCopyrightText: 2018 Scott Harvey + * SPDX-License-Identifier: GPL-3.0-or-later + * License-Filename: LICENSE * */ -#ifndef PERMISSIONS_H -#define PERMISSIONS_H +#ifndef LIBCALAMARES_PERMISSIONS_H +#define LIBCALAMARES_PERMISSIONS_H + +#include "DllMacro.h" #include /** - * @brief The Permissions class takes a QString @p in the form of + * @brief The Permissions class takes a QString @p in the form of * ::, checks it for validity, and makes the three * components available indivdually. */ -class Permissions +class DLLEXPORT Permissions { public: - /** @brief Constructor - * - * Splits the string @p at the colon (":") into separate elements for + * + * Splits the string @p at the colon (":") into separate elements for * , , and (permissions), where is returned as * an **octal** integer. */ - Permissions(QString p); - + Permissions( QString p ); + /** @brief Default constructor of an invalid Permissions. */ Permissions(); @@ -50,13 +40,12 @@ public: QString octal() const { return QString::number( m_value, 8 ); } private: - void parsePermissions(QString const &p); + void parsePermissions( QString const& p ); QString m_username; QString m_group; bool m_valid; int m_value; - }; -#endif // PERMISSIONS_H +#endif // LIBCALAMARES_PERMISSIONS_H diff --git a/src/modules/preservefiles/CMakeLists.txt b/src/modules/preservefiles/CMakeLists.txt index f6cd98008..571de31ca 100644 --- a/src/modules/preservefiles/CMakeLists.txt +++ b/src/modules/preservefiles/CMakeLists.txt @@ -4,7 +4,6 @@ calamares_add_plugin( preservefiles TYPE job EXPORT_MACRO PLUGINDLLEXPORT_PRO SOURCES - permissions.cpp PreserveFiles.cpp LINK_PRIVATE_LIBRARIES calamares diff --git a/src/modules/preservefiles/PreserveFiles.cpp b/src/modules/preservefiles/PreserveFiles.cpp index 175f8e4f8..3e34024e7 100644 --- a/src/modules/preservefiles/PreserveFiles.cpp +++ b/src/modules/preservefiles/PreserveFiles.cpp @@ -1,39 +1,28 @@ /* === This file is part of Calamares - === * - * Copyright 2018, Adriaan de Groot + * SPDX-FileCopyrightText: 2018 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * License-Filename: LICENSE * - * 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 "PreserveFiles.h" -#include "permissions.h" - #include "CalamaresVersion.h" -#include "JobQueue.h" #include "GlobalStorage.h" - +#include "JobQueue.h" #include "utils/CalamaresUtilsSystem.h" #include "utils/CommandList.h" #include "utils/Logger.h" +#include "utils/Permissions.h" #include "utils/Units.h" #include using CalamaresUtils::operator""_MiB; -QString targetPrefix() +QString +targetPrefix() { if ( CalamaresUtils::System::instance()->doChroot() ) { @@ -42,9 +31,13 @@ QString targetPrefix() { QString r = gs->value( "rootMountPoint" ).toString(); if ( !r.isEmpty() ) + { return r; + } else + { cDebug() << "RootMountPoint is empty"; + } } else { @@ -55,16 +48,21 @@ QString targetPrefix() return QLatin1String( "/" ); } -QString atReplacements( QString s ) +QString +atReplacements( QString s ) { Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage(); QString root( "/" ); QString user; if ( gs && gs->contains( "rootMountPoint" ) ) + { root = gs->value( "rootMountPoint" ).toString(); + } if ( gs && gs->contains( "username" ) ) + { user = gs->value( "username" ).toString(); + } return s.replace( "@@ROOT@@", root ).replace( "@@USER@@", user ); } @@ -74,9 +72,7 @@ PreserveFiles::PreserveFiles( QObject* parent ) { } -PreserveFiles::~PreserveFiles() -{ -} +PreserveFiles::~PreserveFiles() {} QString PreserveFiles::prettyName() const @@ -107,8 +103,7 @@ copy_file( const QString& source, const QString& dest ) { b = sourcef.read( 1_MiB ); destf.write( b ); - } - while ( b.count() > 0 ); + } while ( b.count() > 0 ); sourcef.close(); destf.close(); @@ -116,14 +111,19 @@ copy_file( const QString& source, const QString& dest ) return true; } -Calamares::JobResult PreserveFiles::exec() +Calamares::JobResult +PreserveFiles::exec() { if ( m_items.isEmpty() ) + { return Calamares::JobResult::error( tr( "No files configured to save for later." ) ); + } QString prefix = targetPrefix(); if ( !prefix.endsWith( '/' ) ) + { prefix.append( '/' ); + } int count = 0; for ( const auto& it : m_items ) @@ -133,16 +133,24 @@ Calamares::JobResult PreserveFiles::exec() QString dest = prefix + bare_dest; if ( it.type == ItemType::Log ) + { source = Logger::logFile(); + } if ( it.type == ItemType::Config ) { if ( Calamares::JobQueue::instance()->globalStorage()->save( dest ) ) + { cWarning() << "Could not write config for" << dest; + } else + { ++count; + } } else if ( source.isEmpty() ) + { cWarning() << "Skipping unnamed source file for" << dest; + } else { if ( copy_file( source, dest ) ) @@ -153,17 +161,23 @@ Calamares::JobResult PreserveFiles::exec() int r; - r = s_p->targetEnvCall( QStringList{ "chown", it.perm.username(), bare_dest } ); + r = s_p->targetEnvCall( QStringList { "chown", it.perm.username(), bare_dest } ); if ( r ) + { cWarning() << "Could not chown target" << bare_dest; + } - r = s_p->targetEnvCall( QStringList{ "chgrp", it.perm.group(), bare_dest } ); + r = s_p->targetEnvCall( QStringList { "chgrp", it.perm.group(), bare_dest } ); if ( r ) + { cWarning() << "Could not chgrp target" << bare_dest; + } - r = s_p->targetEnvCall( QStringList{ "chmod", it.perm.octal(), bare_dest } ); + r = s_p->targetEnvCall( QStringList { "chmod", it.perm.octal(), bare_dest } ); if ( r ) + { cWarning() << "Could not chmod target" << bare_dest; + } } ++count; @@ -171,12 +185,13 @@ Calamares::JobResult PreserveFiles::exec() } } - return count == m_items.count() ? - Calamares::JobResult::ok() : - Calamares::JobResult::error( tr( "Not all of the configured files could be preserved." ) ); + return count == m_items.count() + ? Calamares::JobResult::ok() + : Calamares::JobResult::error( tr( "Not all of the configured files could be preserved." ) ); } -void PreserveFiles::setConfigurationMap(const QVariantMap& configurationMap) +void +PreserveFiles::setConfigurationMap( const QVariantMap& configurationMap ) { auto files = configurationMap[ "files" ]; if ( !files.isValid() ) @@ -193,7 +208,9 @@ void PreserveFiles::setConfigurationMap(const QVariantMap& configurationMap) QString defaultPermissions = configurationMap[ "perm" ].toString(); if ( defaultPermissions.isEmpty() ) + { defaultPermissions = QStringLiteral( "root:root:0400" ); + } QVariantList l = files.toList(); unsigned int c = 0; @@ -203,22 +220,23 @@ void PreserveFiles::setConfigurationMap(const QVariantMap& configurationMap) { QString filename = li.toString(); if ( !filename.isEmpty() ) - m_items.append( Item{ filename, filename, Permissions( defaultPermissions ), ItemType::Path } ); + m_items.append( Item { filename, filename, Permissions( defaultPermissions ), ItemType::Path } ); else + { cDebug() << "Empty filename for preservefiles, item" << c; + } } else if ( li.type() == QVariant::Map ) { const auto map = li.toMap(); QString dest = map[ "dest" ].toString(); QString from = map[ "from" ].toString(); - ItemType t = - ( from == "log" ) ? ItemType::Log : - ( from == "config" ) ? ItemType::Config : - ItemType::None; + ItemType t = ( from == "log" ) ? ItemType::Log : ( from == "config" ) ? ItemType::Config : ItemType::None; QString perm = map[ "perm" ].toString(); if ( perm.isEmpty() ) + { perm = defaultPermissions; + } if ( dest.isEmpty() ) { @@ -230,15 +248,16 @@ void PreserveFiles::setConfigurationMap(const QVariantMap& configurationMap) } else { - m_items.append( Item{ QString(), dest, Permissions( perm ), t } ); + m_items.append( Item { QString(), dest, Permissions( perm ), t } ); } } else + { cDebug() << "Invalid type for preservefiles, item" << c; + } ++c; } } -CALAMARES_PLUGIN_FACTORY_DEFINITION( PreserveFilesFactory, registerPlugin(); ) - +CALAMARES_PLUGIN_FACTORY_DEFINITION( PreserveFilesFactory, registerPlugin< PreserveFiles >(); ) diff --git a/src/modules/preservefiles/PreserveFiles.h b/src/modules/preservefiles/PreserveFiles.h index 587ac9bab..214ff0df8 100644 --- a/src/modules/preservefiles/PreserveFiles.h +++ b/src/modules/preservefiles/PreserveFiles.h @@ -1,34 +1,22 @@ /* === This file is part of Calamares - === * - * Copyright 2018, Adriaan de Groot + * SPDX-FileCopyrightText: 2018 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * License-Filename: LICENSE * - * 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 PRESERVEFILES_H #define PRESERVEFILES_H -#include -#include -#include - #include "CppJob.h" #include "DllMacro.h" - +#include "utils/Permissions.h" #include "utils/PluginFactory.h" -#include "permissions.h" +#include +#include +#include class PLUGINDLLEXPORT PreserveFiles : public Calamares::CppJob { @@ -40,7 +28,7 @@ class PLUGINDLLEXPORT PreserveFiles : public Calamares::CppJob Path, Log, Config - } ; + }; struct Item { @@ -48,7 +36,7 @@ class PLUGINDLLEXPORT PreserveFiles : public Calamares::CppJob QString dest; Permissions perm; ItemType type; - } ; + }; using ItemList = QList< Item >; @@ -68,4 +56,4 @@ private: CALAMARES_PLUGIN_FACTORY_DECLARATION( PreserveFilesFactory ) -#endif // PRESERVEFILES_H +#endif // PRESERVEFILES_H From 4d3422b93181188bcff06c2938c70748ed9a3a5e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 24 Jul 2020 14:24:03 +0200 Subject: [PATCH 03/13] [libcalamares] dox for Permissions - Expand the documentation, emphasize octal-vs-decimal - east-const consistently in this file (most of Calamares is west-const) - shuffle the is-valid bool to the end of the data members, so sorting by size. --- src/libcalamares/utils/Permissions.cpp | 6 +++--- src/libcalamares/utils/Permissions.h | 29 ++++++++++++++++++++------ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/libcalamares/utils/Permissions.cpp b/src/libcalamares/utils/Permissions.cpp index d9d3226e6..3166d840a 100644 --- a/src/libcalamares/utils/Permissions.cpp +++ b/src/libcalamares/utils/Permissions.cpp @@ -14,20 +14,20 @@ Permissions::Permissions() : m_username() , m_group() - , m_valid( false ) , m_value( 0 ) + , m_valid( false ) { } -Permissions::Permissions( QString p ) +Permissions::Permissions( QString const& p ) : Permissions() { parsePermissions( p ); } void -Permissions::parsePermissions( const QString& p ) +Permissions::parsePermissions( QString const& p ) { QStringList segments = p.split( ":" ); diff --git a/src/libcalamares/utils/Permissions.h b/src/libcalamares/utils/Permissions.h index baa5da554..b6e2d3a44 100644 --- a/src/libcalamares/utils/Permissions.h +++ b/src/libcalamares/utils/Permissions.h @@ -25,27 +25,44 @@ public: /** @brief Constructor * * Splits the string @p at the colon (":") into separate elements for - * , , and (permissions), where is returned as - * an **octal** integer. + * , , and (permissions), where is interpreted + * as an **octal** integer. That is, "root:wheel:755" will give + * you an integer value of four-hundred-ninety-three (493), + * corresponding to the UNIX file permissions rwxr-xr-x, + * as one would expect from chmod and other command-line utilities. */ - Permissions( QString p ); + Permissions( QString const& p ); - /** @brief Default constructor of an invalid Permissions. */ + /// @brief Default constructor of an invalid Permissions. Permissions(); + /// @brief Was the Permissions object constructed from valid data? bool isValid() const { return m_valid; } + /// @brief The user (first component, e.g. "root" in "root:wheel:755") QString username() const { return m_username; } + /// @brief The group (second component, e.g. "wheel" in "root:wheel:755") QString group() const { return m_group; } + /** @brief The value (file permission) as an integer. + * + * Bear in mind that input is in octal, but integers are just integers; + * naively printing them will get decimal results (e.g. 493 from the + * input of "root:wheel:755"). + */ int value() const { return m_value; } - QString octal() const { return QString::number( m_value, 8 ); } + /** @brief The value (file permission) as octal string + * + * This is suitable for passing to chmod-the-program, or for + * recreating the original Permissions string. + */ + QString octal() const { return QString::number( value(), 8 ); } private: void parsePermissions( QString const& p ); QString m_username; QString m_group; - bool m_valid; int m_value; + bool m_valid; }; #endif // LIBCALAMARES_PERMISSIONS_H From bc484ae5da7ee5f52ca661c6e66bfc463d82614c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 22 Jun 2020 16:40:12 +0200 Subject: [PATCH 04/13] [users] Refactor /etc/group file handing --- src/modules/users/CreateUserJob.cpp | 58 +++++++++++++++++------------ 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/src/modules/users/CreateUserJob.cpp b/src/modules/users/CreateUserJob.cpp index 776b45ce9..349d6c295 100644 --- a/src/modules/users/CreateUserJob.cpp +++ b/src/modules/users/CreateUserJob.cpp @@ -54,6 +54,36 @@ CreateUserJob::prettyStatusMessage() const return tr( "Creating user %1." ).arg( m_userName ); } +static QStringList +groupsInTargetSystem( const QDir& targetRoot ) +{ + QFileInfo groupsFi( targetRoot.absoluteFilePath( "etc/group" ) ); + QFile groupsFile( groupsFi.absoluteFilePath() ); + if ( !groupsFile.open( QIODevice::ReadOnly | QIODevice::Text ) ) + { + return QStringList(); + } + QString groupsData = QString::fromLocal8Bit( groupsFile.readAll() ); + QStringList groupsLines = groupsData.split( '\n' ); + for ( QStringList::iterator it = groupsLines.begin(); it != groupsLines.end(); ++it ) + { + int indexOfFirstToDrop = it->indexOf( ':' ); + it->truncate( indexOfFirstToDrop ); + } + return groupsLines; +} + +static void +ensureGroupsExistInTarget( const QStringList& wantedGroups, const QStringList& availableGroups ) +{ + for ( const QString& group : wantedGroups ) + { + if ( !availableGroups.contains( group ) ) + { + CalamaresUtils::System::instance()->targetEnvCall( { "groupadd", group } ); + } + } +} Calamares::JobResult CreateUserJob::exec() @@ -89,36 +119,16 @@ CreateUserJob::exec() cDebug() << "[CREATEUSER]: preparing groups"; - QFileInfo groupsFi( destDir.absoluteFilePath( "etc/group" ) ); - QFile groupsFile( groupsFi.absoluteFilePath() ); - if ( !groupsFile.open( QIODevice::ReadOnly | QIODevice::Text ) ) - { - return Calamares::JobResult::error( tr( "Cannot open groups file for reading." ) ); - } - QString groupsData = QString::fromLocal8Bit( groupsFile.readAll() ); - QStringList groupsLines = groupsData.split( '\n' ); - for ( QStringList::iterator it = groupsLines.begin(); it != groupsLines.end(); ++it ) - { - int indexOfFirstToDrop = it->indexOf( ':' ); - it->truncate( indexOfFirstToDrop ); - } - - for ( const QString& group : m_defaultGroups ) - { - if ( !groupsLines.contains( group ) ) - { - CalamaresUtils::System::instance()->targetEnvCall( { "groupadd", group } ); - } - } + QStringList availableGroups = groupsInTargetSystem( destDir ); + ensureGroupsExistInTarget( m_defaultGroups, availableGroups ); QString defaultGroups = m_defaultGroups.join( ',' ); if ( m_autologin ) { - QString autologinGroup; if ( gs->contains( "autologinGroup" ) && !gs->value( "autologinGroup" ).toString().isEmpty() ) { - autologinGroup = gs->value( "autologinGroup" ).toString(); - CalamaresUtils::System::instance()->targetEnvCall( { "groupadd", autologinGroup } ); + QString autologinGroup = gs->value( "autologinGroup" ).toString(); + ensureGroupsExistInTarget( QStringList { autologinGroup }, availableGroups ); defaultGroups.append( QString( ",%1" ).arg( autologinGroup ) ); } } From 409ab6ee868b22f8e42083ce668876d3f5f313e8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 22 Jun 2020 17:10:03 +0200 Subject: [PATCH 05/13] [users] Refactor writing sudoers file - use existing convenience methods --- src/modules/users/CreateUserJob.cpp | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/modules/users/CreateUserJob.cpp b/src/modules/users/CreateUserJob.cpp index 349d6c295..f0b1dca88 100644 --- a/src/modules/users/CreateUserJob.cpp +++ b/src/modules/users/CreateUserJob.cpp @@ -95,26 +95,23 @@ CreateUserJob::exec() { cDebug() << "[CREATEUSER]: preparing sudoers"; - QFileInfo sudoersFi( destDir.absoluteFilePath( "etc/sudoers.d/10-installer" ) ); + QString sudoersLine = QString( "%%1 ALL=(ALL) ALL\n" ).arg( gs->value( "sudoersGroup" ).toString() ); + auto fileResult + = CalamaresUtils::System::instance()->createTargetFile( QStringLiteral( "/etc/sudoers.d/10-installer" ), + sudoersLine.toUtf8().constData(), + CalamaresUtils::System::WriteMode::Overwrite ); - if ( !sudoersFi.absoluteDir().exists() ) + if ( fileResult ) { - return Calamares::JobResult::error( tr( "Sudoers dir is not writable." ) ); + if ( QProcess::execute( "chmod", { "440", fileResult.path() } ) ) + { + return Calamares::JobResult::error( tr( "Cannot chmod sudoers file." ) ); + } } - - QFile sudoersFile( sudoersFi.absoluteFilePath() ); - if ( !sudoersFile.open( QIODevice::WriteOnly | QIODevice::Text ) ) + else { return Calamares::JobResult::error( tr( "Cannot create sudoers file for writing." ) ); } - - QString sudoersGroup = gs->value( "sudoersGroup" ).toString(); - - QTextStream sudoersOut( &sudoersFile ); - sudoersOut << QString( "%%1 ALL=(ALL) ALL\n" ).arg( sudoersGroup ); - - if ( QProcess::execute( "chmod", { "440", sudoersFi.absoluteFilePath() } ) ) - return Calamares::JobResult::error( tr( "Cannot chmod sudoers file." ) ); } cDebug() << "[CREATEUSER]: preparing groups"; From 1babcd2aa486ead1127c21fff15abee4c48a2c33 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 Jul 2020 10:57:15 +0200 Subject: [PATCH 06/13] [libcalamares] Put Permissions in CalamaresUtils namespace - most of the things in utils/ are in the CalamaresUtils namespace, let Permissions follow suit. Chase the name change in the *preservefiles* module. - add an `apply()` function for doing the most basic of chmod. Note that we don't use `QFile::setPermissions()` because the **values** used are different (0755 for chmod is 0x755 in the enum value passed to `setPermissions()`). --- src/libcalamares/utils/Permissions.cpp | 14 ++++++++++++++ src/libcalamares/utils/Permissions.h | 8 ++++++++ src/modules/preservefiles/PreserveFiles.cpp | 4 ++-- src/modules/preservefiles/PreserveFiles.h | 2 +- 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/libcalamares/utils/Permissions.cpp b/src/libcalamares/utils/Permissions.cpp index 3166d840a..3f5350e86 100644 --- a/src/libcalamares/utils/Permissions.cpp +++ b/src/libcalamares/utils/Permissions.cpp @@ -8,9 +8,14 @@ #include "Permissions.h" +#include + #include #include +namespace CalamaresUtils +{ + Permissions::Permissions() : m_username() , m_group() @@ -64,3 +69,12 @@ Permissions::parsePermissions( QString const& p ) return; } + +bool +Permissions::apply( const QString& path, int mode ) +{ + int r = chmod( path.toUtf8().constData(), mode ); + return r == 0; +} + +} // namespace CalamaresUtils diff --git a/src/libcalamares/utils/Permissions.h b/src/libcalamares/utils/Permissions.h index b6e2d3a44..913d037d5 100644 --- a/src/libcalamares/utils/Permissions.h +++ b/src/libcalamares/utils/Permissions.h @@ -13,6 +13,9 @@ #include +namespace CalamaresUtils +{ + /** * @brief The Permissions class takes a QString @p in the form of * ::, checks it for validity, and makes the three @@ -56,6 +59,9 @@ public: */ QString octal() const { return QString::number( value(), 8 ); } + /// chmod(path, mode), returns true on success + static bool apply( const QString& path, int mode ); + private: void parsePermissions( QString const& p ); @@ -65,4 +71,6 @@ private: bool m_valid; }; +} // namespace CalamaresUtils + #endif // LIBCALAMARES_PERMISSIONS_H diff --git a/src/modules/preservefiles/PreserveFiles.cpp b/src/modules/preservefiles/PreserveFiles.cpp index 3e34024e7..57c2c5422 100644 --- a/src/modules/preservefiles/PreserveFiles.cpp +++ b/src/modules/preservefiles/PreserveFiles.cpp @@ -220,7 +220,7 @@ PreserveFiles::setConfigurationMap( const QVariantMap& configurationMap ) { QString filename = li.toString(); if ( !filename.isEmpty() ) - m_items.append( Item { filename, filename, Permissions( defaultPermissions ), ItemType::Path } ); + m_items.append( Item { filename, filename, CalamaresUtils::Permissions( defaultPermissions ), ItemType::Path } ); else { cDebug() << "Empty filename for preservefiles, item" << c; @@ -248,7 +248,7 @@ PreserveFiles::setConfigurationMap( const QVariantMap& configurationMap ) } else { - m_items.append( Item { QString(), dest, Permissions( perm ), t } ); + m_items.append( Item { QString(), dest, CalamaresUtils::Permissions( perm ), t } ); } } else diff --git a/src/modules/preservefiles/PreserveFiles.h b/src/modules/preservefiles/PreserveFiles.h index 214ff0df8..fdba933fa 100644 --- a/src/modules/preservefiles/PreserveFiles.h +++ b/src/modules/preservefiles/PreserveFiles.h @@ -34,7 +34,7 @@ class PLUGINDLLEXPORT PreserveFiles : public Calamares::CppJob { QString source; QString dest; - Permissions perm; + CalamaresUtils::Permissions perm; ItemType type; }; From 59dff815fcec2247276ddd8264bda029656f2451 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 Jul 2020 11:13:35 +0200 Subject: [PATCH 07/13] [libcalamares] Additional apply() methods for Permissions --- src/libcalamares/utils/Permissions.cpp | 48 ++++++++++++++++++++++++-- src/libcalamares/utils/Permissions.h | 20 +++++++++-- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/libcalamares/utils/Permissions.cpp b/src/libcalamares/utils/Permissions.cpp index 3f5350e86..a7afbc5fc 100644 --- a/src/libcalamares/utils/Permissions.cpp +++ b/src/libcalamares/utils/Permissions.cpp @@ -8,11 +8,14 @@ #include "Permissions.h" -#include +#include "Logger.h" +#include #include #include +#include + namespace CalamaresUtils { @@ -73,8 +76,49 @@ Permissions::parsePermissions( QString const& p ) bool Permissions::apply( const QString& path, int mode ) { - int r = chmod( path.toUtf8().constData(), mode ); + // We **don't** use QFile::setPermissions() here because it takes + // a Qt flags object that subtlely does not align with POSIX bits. + // The Qt flags are **hex** based, so 0x755 for rwxr-xr-x, while + // our integer (mode_t) stores **octal** based flags. + // + // Call chmod(2) directly, that's what Qt would be doing underneath + // anyway. + int r = chmod( path.toUtf8().constData(), mode_t( mode ) ); + if ( r ) + { + cDebug() << Logger::SubEntry << "Could not set permissions of" << path << "to" << QString::number( mode, 8 ); + } return r == 0; } +bool +Permissions::apply( const QString& path, const CalamaresUtils::Permissions& p ) +{ + if ( !p.isValid() ) + { + return false; + } + bool r = apply( path, p.value() ); + if ( r ) + { + // We don't use chgrp(2) or chown(2) here because then we need to + // go through the users list (which one, target or source?) to get + // uid_t and gid_t values to pass to that system call. + // + // Do a lame cop-out and let the chown(8) utility do the heavy lifting. + if ( QProcess::execute( "chown", { p.username() + ':' + p.group(), path } ) ) + { + r = false; + cDebug() << Logger::SubEntry << "Could not set owner of" << path << "to" + << ( p.username() + ':' + p.group() ); + } + } + if ( r ) + { + /* NOTUSED */ apply( path, p.value() ); + } + return r; +} + + } // namespace CalamaresUtils diff --git a/src/libcalamares/utils/Permissions.h b/src/libcalamares/utils/Permissions.h index 913d037d5..1f0dd38da 100644 --- a/src/libcalamares/utils/Permissions.h +++ b/src/libcalamares/utils/Permissions.h @@ -49,7 +49,7 @@ public: * * Bear in mind that input is in octal, but integers are just integers; * naively printing them will get decimal results (e.g. 493 from the - * input of "root:wheel:755"). + * input of "root:wheel:755"). This is suitable to pass to apply(). */ int value() const { return m_value; } /** @brief The value (file permission) as octal string @@ -59,8 +59,24 @@ public: */ QString octal() const { return QString::number( value(), 8 ); } - /// chmod(path, mode), returns true on success + /** @brief Sets the file-access @p mode of @p path + * + * Pass a path that is relative (or absolute) in the **host** system. + */ static bool apply( const QString& path, int mode ); + /** @brief Do both chmod and chown on @p path + * + * Note that interpreting user- and group- names for applying the + * permissions can be different between the host system and the target + * system; the target might not have a "live" user, for instance, and + * the host won't have the user-entered username for the installation. + * + * For this call, the names are interpreted in the **host** system. + * Pass a path that is relative (or absolute) in the **host** system. + */ + static bool apply( const QString& path, const Permissions& p ); + /// Convenience method for apply(const QString&, const Permissions& ) + bool apply( const QString& path ) const { return apply( path, *this ); } private: void parsePermissions( QString const& p ); From 90a0605f38d5268b262998270426b293b0c946e0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 Jul 2020 12:25:50 +0200 Subject: [PATCH 08/13] [preservefiles] [users] Use the Permissions methods - don't call out to tools (executables) when we have an API for it (which might call out to those tools, but that's abstracted) --- src/modules/preservefiles/PreserveFiles.cpp | 21 ++------------------- src/modules/users/CreateUserJob.cpp | 3 ++- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/src/modules/preservefiles/PreserveFiles.cpp b/src/modules/preservefiles/PreserveFiles.cpp index 57c2c5422..8352e861d 100644 --- a/src/modules/preservefiles/PreserveFiles.cpp +++ b/src/modules/preservefiles/PreserveFiles.cpp @@ -157,26 +157,9 @@ PreserveFiles::exec() { if ( it.perm.isValid() ) { - auto s_p = CalamaresUtils::System::instance(); - - int r; - - r = s_p->targetEnvCall( QStringList { "chown", it.perm.username(), bare_dest } ); - if ( r ) - { - cWarning() << "Could not chown target" << bare_dest; - } - - r = s_p->targetEnvCall( QStringList { "chgrp", it.perm.group(), bare_dest } ); - if ( r ) - { - cWarning() << "Could not chgrp target" << bare_dest; - } - - r = s_p->targetEnvCall( QStringList { "chmod", it.perm.octal(), bare_dest } ); - if ( r ) + if ( !it.perm.apply( CalamaresUtils::System::instance()->targetPath( bare_dest ) ) ) { - cWarning() << "Could not chmod target" << bare_dest; + cWarning() << "Could not set attributes of" << bare_dest; } } diff --git a/src/modules/users/CreateUserJob.cpp b/src/modules/users/CreateUserJob.cpp index f0b1dca88..84a42437a 100644 --- a/src/modules/users/CreateUserJob.cpp +++ b/src/modules/users/CreateUserJob.cpp @@ -12,6 +12,7 @@ #include "JobQueue.h" #include "utils/CalamaresUtilsSystem.h" #include "utils/Logger.h" +#include "utils/Permissions.h" #include #include @@ -103,7 +104,7 @@ CreateUserJob::exec() if ( fileResult ) { - if ( QProcess::execute( "chmod", { "440", fileResult.path() } ) ) + if ( CalamaresUtils::Permissions::apply( fileResult.path(), 0440 ) ) { return Calamares::JobResult::error( tr( "Cannot chmod sudoers file." ) ); } From b99b87f787c0e32e1eadb095fafdd45bf8a4f6be Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 Jul 2020 12:37:04 +0200 Subject: [PATCH 09/13] [users] Explain some weird internals --- src/modules/users/CreateUserJob.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/modules/users/CreateUserJob.cpp b/src/modules/users/CreateUserJob.cpp index 84a42437a..2031aa029 100644 --- a/src/modules/users/CreateUserJob.cpp +++ b/src/modules/users/CreateUserJob.cpp @@ -131,7 +131,8 @@ CreateUserJob::exec() } } - // If we're looking to reuse the contents of an existing /home + // If we're looking to reuse the contents of an existing /home. + // This GS setting comes from the **partitioning** module. if ( gs->value( "reuseHome" ).toBool() ) { QString shellFriendlyHome = "/home/" + m_userName; @@ -141,6 +142,7 @@ CreateUserJob::exec() QString backupDirName = "dotfiles_backup_" + QDateTime::currentDateTime().toString( "yyyy-MM-dd_HH-mm-ss" ); existingHome.mkdir( backupDirName ); + // We need the extra `sh -c` here to ensure that we can expand the shell globs CalamaresUtils::System::instance()->targetEnvCall( { "sh", "-c", "mv -f " + shellFriendlyHome + "/.* " + shellFriendlyHome + "/" + backupDirName } ); } From 1fddf723fe30cc2ba35d96a1c11069050c312e00 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 Jul 2020 13:18:09 +0200 Subject: [PATCH 10/13] [users] FreeBSD support creating groups --- src/modules/users/CreateUserJob.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/modules/users/CreateUserJob.cpp b/src/modules/users/CreateUserJob.cpp index 2031aa029..8927af8ab 100644 --- a/src/modules/users/CreateUserJob.cpp +++ b/src/modules/users/CreateUserJob.cpp @@ -81,7 +81,11 @@ ensureGroupsExistInTarget( const QStringList& wantedGroups, const QStringList& a { if ( !availableGroups.contains( group ) ) { +#ifdef __FreeBSD__ + CalamaresUtils::System::instance()->targetEnvCall( { "pw", "groupadd", "-n", group } ); +#else CalamaresUtils::System::instance()->targetEnvCall( { "groupadd", group } ); +#endif } } } From 26b8c826301aa26386e239c4d346f706a60a9e88 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 Jul 2020 13:29:51 +0200 Subject: [PATCH 11/13] [users] Refactor user-creation and user-group-setting into methods - This is prep-work for handling other tools for user- and group- creation as well. --- src/modules/users/CreateUserJob.cpp | 74 ++++++++++++++++++----------- 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/src/modules/users/CreateUserJob.cpp b/src/modules/users/CreateUserJob.cpp index 8927af8ab..e052fa266 100644 --- a/src/modules/users/CreateUserJob.cpp +++ b/src/modules/users/CreateUserJob.cpp @@ -90,6 +90,40 @@ ensureGroupsExistInTarget( const QStringList& wantedGroups, const QStringList& a } } +static Calamares::JobResult +createUser( const QString& loginName, const QString& fullName, const QString& shell ) +{ + QStringList useradd { "useradd", "-m", "-U" }; + if ( !shell.isEmpty() ) + { + useradd << "-s" << shell; + } + useradd << "-c" << fullName; + useradd << loginName; + + auto commandResult = CalamaresUtils::System::instance()->targetEnvCommand( useradd ); + if ( commandResult.getExitCode() ) + { + cError() << "useradd failed" << commandResult.getExitCode(); + return commandResult.explainProcess( useradd, std::chrono::seconds( 10 ) /* bogus timeout */ ); + } + return Calamares::JobResult::ok(); +} + +static Calamares::JobResult +setUserGroups( const QString& loginName, const QStringList& groups ) +{ + auto commandResult + = CalamaresUtils::System::instance()->targetEnvCommand( { "usermod", "-aG", groups.join( ',' ), loginName } ); + if ( commandResult.getExitCode() ) + { + cError() << "usermod failed" << commandResult.getExitCode(); + return commandResult.explainProcess( "usermod", std::chrono::seconds( 10 ) /* bogus timeout */ ); + } + return Calamares::JobResult::ok(); +} + + Calamares::JobResult CreateUserJob::exec() { @@ -122,18 +156,12 @@ CreateUserJob::exec() cDebug() << "[CREATEUSER]: preparing groups"; QStringList availableGroups = groupsInTargetSystem( destDir ); - ensureGroupsExistInTarget( m_defaultGroups, availableGroups ); - - QString defaultGroups = m_defaultGroups.join( ',' ); - if ( m_autologin ) + QStringList groupsForThisUser = m_defaultGroups; + if ( m_autologin && gs->contains( "autologinGroup" ) && !gs->value( "autologinGroup" ).toString().isEmpty() ) { - if ( gs->contains( "autologinGroup" ) && !gs->value( "autologinGroup" ).toString().isEmpty() ) - { - QString autologinGroup = gs->value( "autologinGroup" ).toString(); - ensureGroupsExistInTarget( QStringList { autologinGroup }, availableGroups ); - defaultGroups.append( QString( ",%1" ).arg( autologinGroup ) ); - } + groupsForThisUser << gs->value( "autologinGroup" ).toString(); } + ensureGroupsExistInTarget( m_defaultGroups, availableGroups ); // If we're looking to reuse the contents of an existing /home. // This GS setting comes from the **partitioning** module. @@ -154,33 +182,21 @@ CreateUserJob::exec() cDebug() << "[CREATEUSER]: creating user"; - QStringList useradd { "useradd", "-m", "-U" }; - QString shell = gs->value( "userShell" ).toString(); - if ( !shell.isEmpty() ) - { - useradd << "-s" << shell; - } - useradd << "-c" << m_fullName; - useradd << m_userName; - - auto commandResult = CalamaresUtils::System::instance()->targetEnvCommand( useradd ); - if ( commandResult.getExitCode() ) + auto useraddResult = createUser( m_userName, m_fullName, gs->value( "userShell" ).toString() ); + if ( !useraddResult ) { - cError() << "useradd failed" << commandResult.getExitCode(); - return commandResult.explainProcess( useradd, std::chrono::seconds( 10 ) /* bogus timeout */ ); + return useraddResult; } - commandResult - = CalamaresUtils::System::instance()->targetEnvCommand( { "usermod", "-aG", defaultGroups, m_userName } ); - if ( commandResult.getExitCode() ) + auto usergroupsResult = setUserGroups( m_userName, groupsForThisUser ); + if ( !usergroupsResult ) { - cError() << "usermod failed" << commandResult.getExitCode(); - return commandResult.explainProcess( "usermod", std::chrono::seconds( 10 ) /* bogus timeout */ ); + return usergroupsResult; } QString userGroup = QString( "%1:%2" ).arg( m_userName ).arg( m_userName ); QString homeDir = QString( "/home/%1" ).arg( m_userName ); - commandResult = CalamaresUtils::System::instance()->targetEnvCommand( { "chown", "-R", userGroup, homeDir } ); + auto commandResult = CalamaresUtils::System::instance()->targetEnvCommand( { "chown", "-R", userGroup, homeDir } ); if ( commandResult.getExitCode() ) { cError() << "chown failed" << commandResult.getExitCode(); From 8a6e4af511b0a5294e7236f4309cf2ab1c41cb4d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 Jul 2020 13:45:00 +0200 Subject: [PATCH 12/13] [users] FreeBSD support creating user - call pw useradd and pw usermod as needed; the code paths are basically the same in invoking a program in the target system to do the work. --- src/modules/users/CreateUserJob.cpp | 35 ++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/src/modules/users/CreateUserJob.cpp b/src/modules/users/CreateUserJob.cpp index e052fa266..0fe931fa8 100644 --- a/src/modules/users/CreateUserJob.cpp +++ b/src/modules/users/CreateUserJob.cpp @@ -93,19 +93,33 @@ ensureGroupsExistInTarget( const QStringList& wantedGroups, const QStringList& a static Calamares::JobResult createUser( const QString& loginName, const QString& fullName, const QString& shell ) { - QStringList useradd { "useradd", "-m", "-U" }; + QStringList useraddCommand; +#ifdef __FreeBSD__ + useraddCommand << "pw" + << "useradd" + << "-n" << loginName << "-m" + << "-c" << fullName; + if ( !shell.isEmpty() ) + { + useraddCommand << "-s" << shell; + } +#else + useraddCommand << "useradd" + << "-m" + << "-U"; if ( !shell.isEmpty() ) { useradd << "-s" << shell; } useradd << "-c" << fullName; useradd << loginName; +#endif - auto commandResult = CalamaresUtils::System::instance()->targetEnvCommand( useradd ); + auto commandResult = CalamaresUtils::System::instance()->targetEnvCommand( useraddCommand ); if ( commandResult.getExitCode() ) { cError() << "useradd failed" << commandResult.getExitCode(); - return commandResult.explainProcess( useradd, std::chrono::seconds( 10 ) /* bogus timeout */ ); + return commandResult.explainProcess( useraddCommand, std::chrono::seconds( 10 ) /* bogus timeout */ ); } return Calamares::JobResult::ok(); } @@ -113,12 +127,21 @@ createUser( const QString& loginName, const QString& fullName, const QString& sh static Calamares::JobResult setUserGroups( const QString& loginName, const QStringList& groups ) { - auto commandResult - = CalamaresUtils::System::instance()->targetEnvCommand( { "usermod", "-aG", groups.join( ',' ), loginName } ); + QStringList setgroupsCommand; +#ifdef __FreeBSD__ + setgroupsCommand << "pw" + << "usermod" + << "-n" << loginName << "-G" << groups.join( ',' ); +#else + setgroupsCommand << "usermod" + << "-aG" << groups.join( ',' ) << loginName; +#endif + + auto commandResult = CalamaresUtils::System::instance()->targetEnvCommand( setgroupsCommand ); if ( commandResult.getExitCode() ) { cError() << "usermod failed" << commandResult.getExitCode(); - return commandResult.explainProcess( "usermod", std::chrono::seconds( 10 ) /* bogus timeout */ ); + return commandResult.explainProcess( setgroupsCommand, std::chrono::seconds( 10 ) /* bogus timeout */ ); } return Calamares::JobResult::ok(); } From 8ce7457023aa49bfce2d3291f70612cb2d86958e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 Jul 2020 15:00:14 +0200 Subject: [PATCH 13/13] [users] Add test for create-users code - just one test for groups-file loading - while here fix bug that blank and comment lines were being kept as valid group names --- src/modules/users/CMakeLists.txt | 7 +++++++ src/modules/users/CreateUserJob.cpp | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/modules/users/CMakeLists.txt b/src/modules/users/CMakeLists.txt index 1d969a93b..a449d39ac 100644 --- a/src/modules/users/CMakeLists.txt +++ b/src/modules/users/CMakeLists.txt @@ -49,6 +49,13 @@ calamares_add_test( ${CRYPT_LIBRARIES} ) +calamares_add_test( + userscreatetest + SOURCES + CreateUserTests.cpp + CreateUserJob.cpp +) + calamares_add_test( userstest SOURCES diff --git a/src/modules/users/CreateUserJob.cpp b/src/modules/users/CreateUserJob.cpp index 0fe931fa8..beb454ac0 100644 --- a/src/modules/users/CreateUserJob.cpp +++ b/src/modules/users/CreateUserJob.cpp @@ -55,7 +55,7 @@ CreateUserJob::prettyStatusMessage() const return tr( "Creating user %1." ).arg( m_userName ); } -static QStringList +STATICTEST QStringList groupsInTargetSystem( const QDir& targetRoot ) { QFileInfo groupsFi( targetRoot.absoluteFilePath( "etc/group" ) ); @@ -66,10 +66,22 @@ groupsInTargetSystem( const QDir& targetRoot ) } QString groupsData = QString::fromLocal8Bit( groupsFile.readAll() ); QStringList groupsLines = groupsData.split( '\n' ); - for ( QStringList::iterator it = groupsLines.begin(); it != groupsLines.end(); ++it ) + QStringList::iterator it = groupsLines.begin(); + while ( it != groupsLines.end() ) { + if ( it->startsWith( '#' ) ) + { + it = groupsLines.erase( it ); + continue; + } int indexOfFirstToDrop = it->indexOf( ':' ); + if ( indexOfFirstToDrop < 1 ) + { + it = groupsLines.erase( it ); + continue; + } it->truncate( indexOfFirstToDrop ); + ++it; } return groupsLines; }