Unchecked popen/pclose Return Values

Reviewed-on: #78
Reviewed-by: Hendrik Schutter <localhorst@mosad.xyz>

include/printer.h

@@ -26,11 +26,11 @@ typedef struct
     char caDriveShredDuration[STR_BUFFER_SIZE];
     char caDriveCapacity[STR_BUFFER_SIZE];
     char caDriveState[STR_BUFFER_SIZE];
+    char caDriveConnectionType[STR_BUFFER_SIZE];
     char caDriveModelFamily[STR_BUFFER_SIZE];
     char caDriveModelName[STR_BUFFER_SIZE];
     char caDriveSerialnumber[STR_BUFFER_SIZE];
     char caDriveReHddVersion[STR_BUFFER_SIZE];
-
 } t_driveData;
 
 typedef struct

include/reHDD.h

@@ -8,7 +8,7 @@
 #ifndef REHDD_H_
 #define REHDD_H_
 
-#define REHDD_VERSION "V1.2.0"
+#define REHDD_VERSION "V1.2.1"
 
 // Drive handling Settings
 #define WORSE_HOURS 19200    // mark drive if at this limit or beyond

include/tui.h

@@ -51,6 +51,8 @@ public:
 
     static enum UserInput readUserInput();
 
+    static void terminateTUI();
+
 private:
     static string sCpuUsage;
     static string sRamUsage;
@@ -80,4 +82,4 @@ private:
     string formatSpeed(time_t u32ShredTimeDelta, unsigned long ulWrittenBytes);
     static void vTruncateText(string *psText, uint16_t u16MaxLenght);
 };
-#endif // TUI_H_
+#endif // TUI_H_

src/main.cpp

@@ -16,7 +16,7 @@ int main(void)
 {
     // cout << "refurbishingHddTool" << endl;
 
-    reHDD *app = new reHDD();
-    app->app_logic();
+    reHDD app;
+    app.app_logic();
     return EXIT_SUCCESS;
 }

src/printer.cpp

@@ -53,6 +53,23 @@ void Printer::print(Drive *drive)
     sprintf(msgQueueData.driveData.caDriveErrors, "%i", drive->getErrorCount());
     sprintf(msgQueueData.driveData.caDriveShredTimestamp, "%li", drive->getActionStartTimestamp());
     sprintf(msgQueueData.driveData.caDriveShredDuration, "%li", drive->getTaskDuration());
+
+    switch (drive->connectionType)
+    {
+    case Drive::USB:
+        strcpy(msgQueueData.driveData.caDriveConnectionType, "usb");
+        break;
+    case Drive::SATA:
+        strcpy(msgQueueData.driveData.caDriveConnectionType, "sata");
+        break;
+    case Drive::NVME:
+        strcpy(msgQueueData.driveData.caDriveConnectionType, "nvme");
+        break;
+    case Drive::UNKNOWN:
+    default:
+        strcpy(msgQueueData.driveData.caDriveConnectionType, "na");
+    }
+
     sprintf(msgQueueData.driveData.caDriveReHddVersion, REHDD_VERSION);
 
     if (-1 == msgsnd(this->msqid, &msgQueueData, sizeof(t_msgQueueData) - sizeof(long), 0))

src/reHDD.cpp

@@ -94,7 +94,7 @@ Drive *reHDD::getSelectedDrive()
     else
     {
         Logger::logThis()->warning("selected drive not present");
-        return {};
+        return nullptr;
     }
 }
 
@@ -198,8 +198,9 @@ void reHDD::ThreadUserInput()
             ui->updateTUI(&listDrives, u8SelectedEntry);
             break;
         case TUI::UserInput::Terminate:
-            cout << "Terminate" << endl;
+            // cout << "Terminate" << endl;
             stopShredAllDrives(&listDrives);
+            ui->terminateTUI();
             sleep(5);     // sleep 5 sec
             std::exit(1); // Terminates main, doesn't wait for threads
             break;
@@ -312,6 +313,7 @@ void reHDD::searchDrives(std::list<Drive> *plistDrives)
     if (!fp)
     {
         Logger::logThis()->error("Unable to execute lsblk to scan drives");
+        pclose(fp);
         exit(EXIT_FAILURE);
     }
 
@@ -325,28 +327,28 @@ void reHDD::searchDrives(std::list<Drive> *plistDrives)
         if (devName.empty())
             continue;
 
-        Drive *tmpDrive = new Drive("/dev/" + devName);
-        tmpDrive->state = Drive::NONE;
-        tmpDrive->bIsOffline = false;
+        Drive tmpDrive("/dev/" + devName);
+        tmpDrive.state = Drive::NONE;
+        tmpDrive.bIsOffline = false;
 
         // Set connection type
         if (transport == "sata")
-            tmpDrive->connectionType = Drive::SATA;
+            tmpDrive.connectionType = Drive::SATA;
         else if (transport == "usb")
-            tmpDrive->connectionType = Drive::USB;
+            tmpDrive.connectionType = Drive::USB;
         else if (transport == "nvme")
-            tmpDrive->connectionType = Drive::NVME;
+            tmpDrive.connectionType = Drive::NVME;
         else
-            tmpDrive->connectionType = Drive::UNKNOWN;
+            tmpDrive.connectionType = Drive::UNKNOWN;
 
-        plistDrives->push_back(*tmpDrive);
+        plistDrives->push_back(tmpDrive);
 
         Logger::logThis()->info(
-            "Drive found: " + tmpDrive->getPath() +
+            "Drive found: " + tmpDrive.getPath() +
             " (type: " +
-            (tmpDrive->connectionType == Drive::USB ? "USB" : tmpDrive->connectionType == Drive::SATA ? "SATA"
-                                                          : tmpDrive->connectionType == Drive::NVME   ? "NVME"
-                                                                                                      : "UNKNOWN") +
+            (tmpDrive.connectionType == Drive::USB ? "USB" : tmpDrive.connectionType == Drive::SATA ? "SATA"
+                                                         : tmpDrive.connectionType == Drive::NVME   ? "NVME"
+                                                                                                    : "UNKNOWN") +
             ")");
     }
 
@@ -403,15 +405,17 @@ void reHDD::filterIgnoredDrives(list<Drive> *plistDrives)
             FILE *outputfileBlkid = popen(sCMD.c_str(), "r"); // get UUID from drive
             if (outputfileBlkid == NULL)
             {
+                pclose(outputfileBlkid);
                 exit(EXIT_FAILURE);
             }
 
             while ((getline(&cLine, &len, outputfileBlkid)) != -1) // parse UUID from blkid
             {
-                if (string(cLine).find("PTUUID") != string::npos)
+                size_t ptuuidPos = string(cLine).find("PTUUID");
+                if (ptuuidPos != string::npos)
                 {
                     string sBlkidOut = string(cLine);
-                    sBlkidOut.erase(0, 18);
+                    sBlkidOut.erase(0, ptuuidPos + 8);
                     sBlkidOut.erase(8, sBlkidOut.length());
                     sUUID = sBlkidOut;
                     // cout << "blkid uuid:" << sUUID << endl;
@@ -681,6 +685,7 @@ bool reHDD::getSystemDrive(string &systemDrive)
     if (outputfileHwinfo == NULL)
     {
         Logger::logThis()->error("Unable to scan attached drives for system drive");
+        pclose(outputfileHwinfo);
         exit(EXIT_FAILURE);
     }
 
@@ -724,4 +729,4 @@ bool reHDD::getSystemDrive(string &systemDrive)
     pclose(outputfileHwinfo);
 
     return systemDriveFound;
-}
+}

src/tui.cpp

@@ -63,6 +63,11 @@ void TUI::initTUI()
 
 void TUI::updateTUI(list<Drive> *plistDrives, uint8_t u8SelectedEntry)
 {
+    if (isendwin())
+    {
+        return;
+    }
+
     mxUIrefresh.lock();
     uint16_t u16StdscrX, u16StdscrY;
     getmaxyx(stdscr, u16StdscrY, u16StdscrX);
@@ -250,6 +255,11 @@ enum TUI::UserInput TUI::readUserInput()
     return TUI::UserInput::Undefined;
 }
 
+void TUI::terminateTUI()
+{
+    endwin();
+}
+
 void TUI::centerTitle(WINDOW *pwin, const char *title)
 {
     int x, maxX, stringSize;