From cac07c1472fdbec47eaa05458598c13e3a83b011 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 1 Aug 2019 22:47:42 +0200 Subject: [PATCH] [libcalamares] Use std::chrono::seconds for timeouts - Distinguish just-an-int from seconds all across the API --- src/libcalamares/ProcessJob.cpp | 2 +- src/libcalamares/ProcessJob.h | 6 ++-- src/libcalamares/PythonJobApi.cpp | 4 ++- .../utils/CalamaresUtilsSystem.cpp | 22 +++++++-------- src/libcalamares/utils/CalamaresUtilsSystem.h | 28 +++++++++---------- src/libcalamares/utils/CommandList.cpp | 12 ++++---- src/libcalamares/utils/CommandList.h | 22 ++++++++------- 7 files changed, 51 insertions(+), 45 deletions(-) diff --git a/src/libcalamares/ProcessJob.cpp b/src/libcalamares/ProcessJob.cpp index 47fcee05d..f952fefe6 100644 --- a/src/libcalamares/ProcessJob.cpp +++ b/src/libcalamares/ProcessJob.cpp @@ -31,7 +31,7 @@ namespace Calamares { ProcessJob::ProcessJob( const QString& command, const QString& workingPath, bool runInChroot, - int secondsTimeout, + std::chrono::seconds secondsTimeout, QObject* parent ) : Job( parent ) , m_command( command ) diff --git a/src/libcalamares/ProcessJob.h b/src/libcalamares/ProcessJob.h index 224ebdaf0..84f84e550 100644 --- a/src/libcalamares/ProcessJob.h +++ b/src/libcalamares/ProcessJob.h @@ -22,6 +22,8 @@ #include "Job.h" +#include + namespace Calamares { class ProcessJob : public Job @@ -31,7 +33,7 @@ public: explicit ProcessJob( const QString& command, const QString& workingPath, bool runInChroot = false, - int secondsTimeout = 30, + std::chrono::seconds secondsTimeout = std::chrono::seconds(30), QObject* parent = nullptr ); virtual ~ProcessJob() override; @@ -43,7 +45,7 @@ private: QString m_command; QString m_workingPath; bool m_runInChroot; - int m_timeoutSec; + std::chrono::seconds m_timeoutSec; }; } // namespace Calamares diff --git a/src/libcalamares/PythonJobApi.cpp b/src/libcalamares/PythonJobApi.cpp index 8e8b8b2ab..750d8ea73 100644 --- a/src/libcalamares/PythonJobApi.cpp +++ b/src/libcalamares/PythonJobApi.cpp @@ -89,11 +89,13 @@ _target_env_command( const std::string& stdin, int timeout ) { + // Since Python doesn't give us the type system for distinguishing + // seconds from other integral types, massage to seconds here. return CalamaresUtils::System::instance()-> targetEnvCommand( args, QString(), QString::fromStdString( stdin ), - timeout ); + std::chrono::seconds( timeout ) ); } int diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index 4fe571e6d..894c434be 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -219,7 +219,7 @@ System::runCommand( } process.closeWriteChannel(); - if ( !process.waitForFinished( timeoutSec ? ( timeoutSec * 1000 ) : -1 ) ) + if ( !process.waitForFinished( timeoutSec > std::chrono::seconds::zero() ? ( std::chrono::milliseconds( timeoutSec ).count() ) : -1 ) ) { cWarning().noquote().nospace() << "Timed out. Output so far:\n" << process.readAllStandardOutput(); @@ -249,7 +249,7 @@ QString System::targetPath( const QString& path ) const { QString completePath; - + if ( doChroot() ) { Calamares::GlobalStorage* gs = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr; @@ -259,14 +259,14 @@ System::targetPath( const QString& path ) const cWarning() << "No rootMountPoint in global storage, cannot create target file" << path; return QString(); } - + completePath = gs->value( "rootMountPoint" ).toString() + '/' + path; } else { completePath = QStringLiteral( "/" ) + path; } - + return completePath; } @@ -278,32 +278,32 @@ System::createTargetFile( const QString& path, const QByteArray& contents ) cons { return QString(); } - + QFile f( completePath ); if ( f.exists() ) { return QString(); } - + QIODevice::OpenMode m = #if QT_VERSION >= QT_VERSION_CHECK( 5, 11, 0 ) // New flag from Qt 5.11, implies WriteOnly QIODevice::NewOnly | #endif QIODevice::WriteOnly | QIODevice::Truncate; - + if ( !f.open( m ) ) { return QString(); } - + if ( f.write( contents ) != contents.size() ) { f.close(); f.remove(); return QString(); } - + f.close(); return QFileInfo( f ).canonicalFilePath(); } @@ -371,7 +371,7 @@ System::doChroot() const } Calamares::JobResult -ProcessResult::explainProcess( int ec, const QString& command, const QString& output, int timeout ) +ProcessResult::explainProcess( int ec, const QString& command, const QString& output, std::chrono::seconds timeout ) { using Calamares::JobResult; @@ -401,7 +401,7 @@ ProcessResult::explainProcess( int ec, const QString& command, const QString& ou return JobResult::error( QCoreApplication::translate( "ProcessResult", "External command failed to finish." ), QCoreApplication::translate( "ProcessResult", "Command %1 failed to finish in %2 seconds." ) .arg( command ) - .arg( timeout ) + .arg( timeout.count() ) + outputMessage ); //Any other exit code diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.h b/src/libcalamares/utils/CalamaresUtilsSystem.h index d39bcaab1..5b10f7d1a 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.h +++ b/src/libcalamares/utils/CalamaresUtilsSystem.h @@ -62,16 +62,16 @@ public: * @param timeout Timeout passed to the process runner, for explaining * error code -4 (timeout). */ - static Calamares::JobResult explainProcess( int errorCode, const QString& command, const QString& output, int timeout ); + static Calamares::JobResult explainProcess( int errorCode, const QString& command, const QString& output, std::chrono::seconds timeout ); /// @brief Convenience wrapper for explainProcess() - inline Calamares::JobResult explainProcess( const QString& command, int timeout ) const + inline Calamares::JobResult explainProcess( const QString& command, std::chrono::seconds timeout ) const { return explainProcess( getExitCode(), command, getOutput(), timeout ); } /// @brief Convenience wrapper for explainProcess() - inline Calamares::JobResult explainProcess( const QStringList& command, int timeout ) const + inline Calamares::JobResult explainProcess( const QStringList& command, std::chrono::seconds timeout ) const { return explainProcess( getExitCode(), command.join( ' ' ), getOutput(), timeout ); } @@ -207,40 +207,40 @@ public: return targetEnvOutput( QStringList{ command }, output, workingPath, stdInput, timeoutSec ); } - + /** @brief Gets a path to a file in the target system, from the host. - * + * * @param path Path to the file; this is interpreted * from the root of the target system (whatever that may be, * but / in the chroot, or / in OEM modes). - * + * * @return The complete path to the target file, from * the root of the host system, or empty on failure. - * + * * For instance, during installation where the target root is * mounted on /tmp/calamares-something, asking for targetPath("/etc/passwd") * will give you /tmp/calamares-something/etc/passwd. - * + * * No attempt is made to canonicalize anything, since paths might not exist. - */ + */ DLLEXPORT QString targetPath( const QString& path ) const; - + /** @brief Create a (small-ish) file in the target system. - * + * * @param path Path to the file; this is interpreted * from the root of the target system (whatever that may be, * but / in the chroot, or / in OEM modes). * @param contents Actual content of the file. - * + * * Will not overwrite files. Returns an empty string if the * target file already exists. - * + * * @return The complete canonical path to the target file from the * root of the host system, or empty on failure. (Here, it is * possible to be canonical because the file exists). */ DLLEXPORT QString createTargetFile( const QString& path, const QByteArray& contents ) const; - + /** * @brief getTotalMemoryB returns the total main memory, in bytes. * diff --git a/src/libcalamares/utils/CommandList.cpp b/src/libcalamares/utils/CommandList.cpp index 9916c71fe..414cfa9e5 100644 --- a/src/libcalamares/utils/CommandList.cpp +++ b/src/libcalamares/utils/CommandList.cpp @@ -35,10 +35,10 @@ namespace CalamaresUtils static CommandLine get_variant_object( const QVariantMap& m ) { QString command = CalamaresUtils::getString( m, "command" ); - int timeout = CalamaresUtils::getInteger( m, "timeout", CommandLine::TimeoutNotSet ); + int timeout = CalamaresUtils::getInteger( m, "timeout", -1 ); if ( !command.isEmpty() ) - return CommandLine( command, timeout ); + return CommandLine( command, timeout >= 0 ? std::chrono::seconds( timeout ) : CommandLine::TimeoutNotSet() ); cWarning() << "Bad CommandLine element" << m; return CommandLine(); } @@ -50,7 +50,7 @@ static CommandList_t get_variant_stringlist( const QVariantList& l ) for ( const auto& v : l ) { if ( v.type() == QVariant::String ) - retl.append( CommandLine( v.toString(), CommandLine::TimeoutNotSet ) ); + retl.append( CommandLine( v.toString(), CommandLine::TimeoutNotSet() ) ); else if ( v.type() == QVariant::Map ) { auto command( get_variant_object( v.toMap() ) ); @@ -65,13 +65,13 @@ static CommandList_t get_variant_stringlist( const QVariantList& l ) return retl; } -CommandList::CommandList( bool doChroot, int timeout ) +CommandList::CommandList( bool doChroot, std::chrono::seconds timeout ) : m_doChroot( doChroot ) , m_timeout( timeout ) { } -CommandList::CommandList::CommandList( const QVariant& v, bool doChroot, int timeout ) +CommandList::CommandList::CommandList( const QVariant& v, bool doChroot, std::chrono::seconds timeout ) : CommandList( doChroot, timeout ) { if ( v.type() == QVariant::List ) @@ -155,7 +155,7 @@ Calamares::JobResult CommandList::run() QStringList shell_cmd { "/bin/sh", "-c" }; shell_cmd << processed_cmd; - int timeout = i->timeout() >= 0 ? i->timeout() : m_timeout; + std::chrono::seconds timeout = i->timeout() >= std::chrono::seconds::zero() ? i->timeout() : m_timeout; ProcessResult r = System::runCommand( location, shell_cmd, QString(), QString(), timeout ); diff --git a/src/libcalamares/utils/CommandList.h b/src/libcalamares/utils/CommandList.h index 3dccdec6a..7453ae6ce 100644 --- a/src/libcalamares/utils/CommandList.h +++ b/src/libcalamares/utils/CommandList.h @@ -24,6 +24,8 @@ #include #include +#include + namespace CalamaresUtils { @@ -31,23 +33,23 @@ namespace CalamaresUtils * Each command can have an associated timeout in seconds. The timeout * defaults to 10 seconds. Provide some convenience naming and construction. */ -struct CommandLine : public QPair< QString, int > +struct CommandLine : public QPair< QString, std::chrono::seconds > { - enum { TimeoutNotSet = -1 }; + static inline constexpr std::chrono::seconds TimeoutNotSet() { return std::chrono::seconds(-1); } /// An invalid command line CommandLine() - : QPair< QString, int >( QString(), TimeoutNotSet ) + : QPair( QString(), TimeoutNotSet() ) { } CommandLine( const QString& s ) - : QPair< QString, int >( s, TimeoutNotSet ) + : QPair( s, TimeoutNotSet() ) { } - CommandLine( const QString& s, int t ) - : QPair< QString, int >( s, t) + CommandLine( const QString& s, std::chrono::seconds t ) + : QPair( s, t) { } @@ -56,7 +58,7 @@ struct CommandLine : public QPair< QString, int > return first; } - int timeout() const + std::chrono::seconds timeout() const { return second; } @@ -82,8 +84,8 @@ class CommandList : protected CommandList_t { public: /** @brief empty command-list with timeout to apply to entries. */ - CommandList( bool doChroot = true, int timeout = 10 ); - CommandList( const QVariant& v, bool doChroot = true, int timeout = 10 ); + CommandList( bool doChroot = true, std::chrono::seconds timeout = std::chrono::seconds( 10 ) ); + CommandList( const QVariant& v, bool doChroot = true, std::chrono::seconds timeout = std::chrono::seconds( 10 ) ); ~CommandList(); bool doChroot() const @@ -106,7 +108,7 @@ protected: private: bool m_doChroot; - int m_timeout; + std::chrono::seconds m_timeout; } ; } // namespace