[libcalamares] Use std::chrono::seconds for timeouts

- Distinguish just-an-int from seconds all across the API

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 )

src/libcalamares/ProcessJob.h

@@ -22,6 +22,8 @@
 
 #include "Job.h"
 
+#include <chrono>
+
 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

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

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 <i>%1</i> failed to finish in %2 seconds." )
                                     .arg( command )
-                                    .arg( timeout )
+                                    .arg( timeout.count() )
                                     + outputMessage );
 
     //Any other exit code

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.
      *

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 );
 

src/libcalamares/utils/CommandList.h

@@ -24,6 +24,8 @@
 #include <QStringList>
 #include <QVariant>
 
+#include <chrono>
+
 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 )
+    CommandLine( const QString& s, std::chrono::seconds t )
-        : QPair< QString, int >( s, 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( bool doChroot = true, std::chrono::seconds timeout = std::chrono::seconds( 10 ) );
-    CommandList( const QVariant& v, bool doChroot = true, int timeout = 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