improved error handling of invalid ExecuteJobs

CMakeLists.txt

@@ -7,8 +7,8 @@ project(fancontroller)
 option(NO_SYSTEMD "Compile without Systemd support. Reduces functionality significantly!" OFF)
 option(BUILD_GUI "Build the standalone application" ON)
 option(BUILD_KCM "Build the KCM" OFF)
+option(BUILD_HELPER "Build the KHelper" ON)
 option(INSTALL_SHARED "Install the shared parts" ON)
-option(INSTALL_HELPER "Install the KHelper" ON)
 
 #variables
 set(STANDARD_SERVICE_NAME "fancontrol" CACHE STRING "The name of the systemd service for the fancontrol script")
@@ -70,11 +70,11 @@ endif(NOT NO_SYSTEMD)
 
 
 #KHelper for actions that require superuser rights
-if(INSTALL_HELPER)
+if(BUILD_HELPER)
 
     add_subdirectory(helper)
 
-endif(INSTALL_HELPER)
+endif(BUILD_HELPER)
 
 
 #Build the standalone application
@@ -95,7 +95,7 @@ if(BUILD_KCM)
 endif(BUILD_KCM)
 
 
-#install the shared parts
+#build and install the shared parts
 if(INSTALL_SHARED)
 
     #qml plugin

import/src/loader.cpp

@@ -267,33 +267,39 @@ bool Loader::load(const QUrl &url)
     else if (file.exists())
     {
         KAuth::Action action = newFancontrolAction();
-        QVariantMap map;
-        map[QStringLiteral("action")] = QVariant("read");
-        map[QStringLiteral("filename")] = fileName;
-        action.setArguments(map);
-        KAuth::ExecuteJob *reply = action.execute();
-        if (!reply->exec())
+        
+        if (action.isValid())
         {
-            if (reply->error() == 4)
+            QVariantMap map;
+            map[QStringLiteral("action")] = QVariant("read");
+            map[QStringLiteral("filename")] = fileName;
+            action.setArguments(map);
+            KAuth::ExecuteJob *reply = action.execute();
+            if (!reply->exec())
             {
-                qDebug() << "Aborted by user";
+                if (reply->error() == 4)
+                {
+                    qDebug() << "Aborted by user";
+                    return false;
+                }
+                
+                qDebug() << "Error while loading:" << reply->error();
+                setError(reply->errorString() + reply->errorText(), true);
                 return false;
             }
-            
-            qDebug() << "Error while loading:" << reply->error();
-            setError(reply->errorString() + reply->errorText(), true);
-            return false;
-        }
-        else
-        {
-            if (!url.isEmpty())
+            else
             {
-                m_configUrl = url;
-                emit configUrlChanged();
-            }
+                if (!url.isEmpty())
+                {
+                    m_configUrl = url;
+                    emit configUrlChanged();
+                }
 
-            fileContent = reply->data().value(QStringLiteral("content")).toString();
+                fileContent = reply->data().value(QStringLiteral("content")).toString();
+            }
         }
+        else
+            setError(i18n("Action not supported! Try running the application as root."), true);
     }
     else
     {
@@ -493,26 +499,32 @@ bool Loader::save(const QUrl &url)
     else
     {
         KAuth::Action action = newFancontrolAction();
-        QVariantMap map;
-        map[QStringLiteral("action")] = QVariant("write");
-        map[QStringLiteral("filename")] = fileName;
-        map[QStringLiteral("content")] = m_configFile;
+        
+        if (action.isValid())
+        {
+            QVariantMap map;
+            map[QStringLiteral("action")] = QVariant("write");
+            map[QStringLiteral("filename")] = fileName;
+            map[QStringLiteral("content")] = m_configFile;
 
-        action.setArguments(map);
-        KAuth::ExecuteJob *reply = action.execute();
+            action.setArguments(map);
+            KAuth::ExecuteJob *reply = action.execute();
 
-        if (!reply->exec())
-        {
-            if (reply->error() == 4)
+            if (!reply->exec())
             {
-                qDebug() << "Aborted by user";
+                if (reply->error() == 4)
+                {
+                    qDebug() << "Aborted by user";
+                    return false;
+                }
+                
+                qDebug() << "Error while saving:" << reply->error();
+                setError(reply->errorString() + reply->errorText(), true);
                 return false;
             }
-            
-            qDebug() << "Error while saving:" << reply->error();
-            setError(reply->errorString() + reply->errorText(), true);
-            return false;
         }
+        else
+            setError(i18n("Action not supported! Try running the application as root."), true);
     }
 
     return true;
@@ -697,14 +709,20 @@ void Loader::handleDetectSensorsResult(int exitCode)
             setError(process->readAllStandardOutput());
         
         KAuth::Action action = newFancontrolAction();
-        QVariantMap map;
-        map[QStringLiteral("action")] = QVariant("detectSensors");
-        
-        action.setArguments(map);
-        KAuth::ExecuteJob *job = action.execute();
         
-        connect(job, &KAuth::ExecuteJob::result, this, static_cast<void(Loader::*)(KJob *)>(&Loader::handleDetectSensorsResult));
-        job->start();
+        if (action.isValid())
+        {
+            QVariantMap map;
+            map[QStringLiteral("action")] = QVariant("detectSensors");
+            
+            action.setArguments(map);
+            KAuth::ExecuteJob *job = action.execute();
+            
+            connect(job, &KAuth::ExecuteJob::result, this, static_cast<void(Loader::*)(KJob *)>(&Loader::handleDetectSensorsResult));
+            job->start();
+        }
+        else
+            setError(i18n("Action not supported! Try running the application as root."), true);
     }
     else
     {

import/src/pwmfan.cpp

@@ -177,25 +177,31 @@ bool PwmFan::setPwm(int pwm, bool write)
             else
             {
                 KAuth::Action action = newFancontrolAction();
-                QVariantMap map;
-                map[QStringLiteral("action")] = "write";
-                map[QStringLiteral("filename")] = qobject_cast<QFile *>(m_pwmStream->device())->fileName();
-                map[QStringLiteral("content")] = QString::number(pwm);
-                action.setArguments(map);
                 
-                KAuth::ExecuteJob *job = action.execute();
-                if (!job->exec())
+                if (action.isValid())
                 {
-                    if (job->error() == KAuth::ActionReply::HelperBusyError)
+                    QVariantMap map;
+                    map[QStringLiteral("action")] = "write";
+                    map[QStringLiteral("filename")] = qobject_cast<QFile *>(m_pwmStream->device())->fileName();
+                    map[QStringLiteral("content")] = QString::number(pwm);
+                    action.setArguments(map);
+                    
+                    KAuth::ExecuteJob *job = action.execute();
+                    if (!job->exec())
                     {
-                        qDebug() << "Helper busy...";
+                        if (job->error() == KAuth::ActionReply::HelperBusyError)
+                        {
+                            qDebug() << "Helper busy...";
+                            
+                            QTimer::singleShot(50, this, [this] (){ setPwmMode(m_pwmMode); });
+                        }
                         
-                        QTimer::singleShot(50, this, [this] (){ setPwmMode(m_pwmMode); });
+                        emit errorChanged(i18n("Could not set pwm: ") + job->errorText());
                     }
-                    
-                    emit errorChanged(i18n("Could not set pwm: ") + job->errorText());
+                    update();
                 }
-                update();
+                else
+                    emit errorChanged(i18n("Action not supported! Try running the application as root."), true);
             }
         }
     }
@@ -218,25 +224,30 @@ bool PwmFan::setPwmMode(int pwmMode, bool write)
             {
                 KAuth::Action action = newFancontrolAction();
                 
-                QVariantMap map;
-                map[QStringLiteral("action")] = QVariant("write");
-                map[QStringLiteral("filename")] = qobject_cast<QFile *>(m_modeStream->device())->fileName();
-                map[QStringLiteral("content")] = QString::number(pwmMode);
-                action.setArguments(map);
-
-                KAuth::ExecuteJob *job = action.execute();
-                if (!job->exec())
+                if (action.isValid())
                 {
-                    if (job->error() == KAuth::ActionReply::HelperBusyError)
+                    QVariantMap map;
+                    map[QStringLiteral("action")] = QVariant("write");
+                    map[QStringLiteral("filename")] = qobject_cast<QFile *>(m_modeStream->device())->fileName();
+                    map[QStringLiteral("content")] = QString::number(pwmMode);
+                    action.setArguments(map);
+
+                    KAuth::ExecuteJob *job = action.execute();
+                    if (!job->exec())
                     {
-                        qDebug() << "Helper busy...";
+                        if (job->error() == KAuth::ActionReply::HelperBusyError)
+                        {
+                            qDebug() << "Helper busy...";
+                            
+                            QTimer::singleShot(50, this, [this] (){ setPwmMode(m_pwmMode); });
+                        }
                         
-                        QTimer::singleShot(50, this, [this] (){ setPwmMode(m_pwmMode); });
+                        emit errorChanged(i18n("Could not set pwm mode: ") + job->errorText());
                     }
-                    
-                    emit errorChanged(i18n("Could not set pwm mode: ") + job->errorText());
+                    update();
                 }
-                update();
+                else
+                    emit errorChanged(i18n("Action not supported! Try running the application as root."), true);
             }
         }
     }
@@ -248,21 +259,30 @@ void PwmFan::test()
     if (!m_modeStream->device()->isWritable() || !m_pwmStream->device()->isWritable())
     {
         KAuth::Action action = newFancontrolAction();
-        KAuth::ExecuteJob *job = action.execute();
-
-        if (!job->exec())
+        
+        if (action.isValid())
         {
-            if (job->error() == KAuth::ActionReply::HelperBusyError)
+            KAuth::ExecuteJob *job = action.execute();
+
+            if (!job->exec())
             {
-                qDebug() << "Helper busy...";
+                if (job->error() == KAuth::ActionReply::HelperBusyError)
+                {
+                    qDebug() << "Helper busy...";
+                    
+                    QTimer::singleShot(100, this, &PwmFan::test);
+                    return;
+                }
                 
-                QTimer::singleShot(100, this, &PwmFan::test);
+                emit errorChanged(i18n("Authorization error: ") + job->errorText());
+                m_testStatus = Error;
+                emit testStatusChanged();
                 return;
             }
-            
-            emit errorChanged(i18n("Authorization error: ") + job->errorText());
-            m_testStatus = Error;
-            emit testStatusChanged();
+        }
+        else
+        {
+            emit errorChanged(i18n("Action not supported! Try running the application as root."), true);
             return;
         }
     }