100% code coverage and misc fixes

bindings/python/example.py

@@ -21,6 +21,7 @@ libipc = ctypes.CDLL(os.path.join(os.path.dirname(os.path.abspath(__file__)),lib
 # we create a new PCSX2Ipc object
 ipc = libipc.pcsx2ipc_new()
 
+# TODO: fix the batchcommand handles in the bindings
 libipc.pcsx2ipc_initialize_batch(ipc)
 libipc.write(ipc, 0x00347D34, 0xFF, 4, True)
 res = libipc.pcsx2ipc_finalize_batch(ipc)

meson.build

@@ -14,7 +14,8 @@ if catch2.found()
   # TODO: in the future if we need to add threads to the API test cases might
   # run into an infinite loop, i have absolutely no clue why that happens but
   # including thread_dep here just leads to it. so don't include that future me.
-  e = executable('tests', test_src, dependencies : [catch2, winsock])
+  e = executable('tests', test_src, dependencies : [catch2, winsock], cpp_args :
+    '-DTESTS')
   test('tests', e)
 endif
 

src/pcsx2_ipc.h

@@ -47,6 +47,8 @@
  * -Gauvain "GovanifY" Roussel-Tarbouriech, 2020
  */
 class PCSX2Ipc {
+
+    // allow test suite to poke internals
   protected:
 #if defined(_WIN32) || defined(DOXYGEN)
     /**
@@ -71,7 +73,7 @@ class PCSX2Ipc {
      * @see MAX_IPC_RETURN_SIZE
      * @see MAX_BATCH_REPLY_COUNT
      */
-    const int MAX_IPC_SIZE = 650000;
+#define MAX_IPC_SIZE 650000
 
     /**
      * Maximum memory used by an IPC message reply.
@@ -79,14 +81,14 @@ class PCSX2Ipc {
      * @see MAX_IPC_SIZE
      * @see MAX_BATCH_REPLY_COUNT
      */
-    const unsigned int MAX_IPC_RETURN_SIZE = 450000;
+#define MAX_IPC_RETURN_SIZE 450000
 
     /**
      * Maximum number of commands sent in a batch message.
      * @see MAX_IPC_RETURN_SIZE
      * @see MAX_IPC_SIZE
      */
-    const unsigned int MAX_BATCH_REPLY_COUNT = 50000;
+#define MAX_BATCH_REPLY_COUNT 50000
 
     /**
      * IPC return buffer. @n
@@ -111,7 +113,7 @@ class PCSX2Ipc {
      * @see IPCCommand
      * @see MAX_IPC_SIZE
      */
-    uint16_t batch_len = 0;
+    unsigned int batch_len = 0;
 
     /**
      * Length of the reply of the batch IPC request. @n
@@ -340,8 +342,8 @@ class PCSX2Ipc {
      */
     template <IPCCommand T, typename Y>
     auto GetReply(const Y &cmd, int place) {
-        char *buf;
-        int loc;
+        [[maybe_unused]] char *buf;
+        [[maybe_unused]] int loc;
         if constexpr (std::is_same<Y, BatchCommand>::value) {
             buf = cmd.ipc_return.buffer;
             loc = cmd.return_locations[place];
@@ -389,10 +391,6 @@ class PCSX2Ipc {
         struct sockaddr_in server;
 
         sock = socket(AF_INET, SOCK_STREAM, 0);
-        if (sock < 0) {
-            SetError(Fail);
-            return;
-        }
 
         // Prepare the sockaddr_in structure
         server.sin_family = AF_INET;
@@ -411,10 +409,6 @@ class PCSX2Ipc {
         struct sockaddr_un server;
 
         sock = socket(AF_UNIX, SOCK_STREAM, 0);
-        if (sock < 0) {
-            SetError(Fail);
-            return;
-        }
         server.sun_family = AF_UNIX;
         strcpy(server.sun_path, SOCKET_NAME);
 
@@ -471,7 +465,10 @@ class PCSX2Ipc {
     }
 
     /**
-     * Finalizes a MsgMultiCommand IPC message.
+     * Finalizes a MsgMultiCommand IPC message. @n
+     * WARNING: You will ALWAYS have to call a FinalizeBatch, even on
+     * exceptions, once an InitializeBatch has been called overthise the class
+     * will deadlock.
      * @return A BatchCommand with:
      *         * The IPCBuffer of the message.
      *         * The IPCBuffer of the return.
@@ -561,9 +558,10 @@ class PCSX2Ipc {
         } else {
             // we are already locked in batch mode
             std::lock_guard<std::mutex> lock(ipc_blocking);
-            SendCommand(
-                IPCBuffer{ 5, FormatBeginning(ipc_buffer, address, tag) },
-                IPCBuffer{ 1 + sizeof(Y), ret_buffer });
+            IPCBuffer cmd =
+                IPCBuffer{ 5, FormatBeginning(ipc_buffer, address, tag) };
+            IPCBuffer ret = IPCBuffer{ 1 + sizeof(Y), ret_buffer };
+            SendCommand(cmd, ret);
             return GetReply<tag>(ret_buffer, 1);
         }
     }

src/tests.cpp

@@ -1,6 +1,7 @@
 #include "pcsx2_ipc.h"
 #define CATCH_CONFIG_MAIN
 #include <catch2/catch.hpp>
+#include <climits>
 
 #define u8 uint8_t
 #define u16 uint16_t
@@ -28,14 +29,25 @@ auto open_pcsx2() -> void {
         char pcsx2_path[4096];
         strcpy(pcsx2_path, env_p);
         strcat(pcsx2_path, " &");
-        (void)system(pcsx2_path);
+        if (system(pcsx2_path) != 0) {
+            printf("PCSX2 failed to execute!\n");
+        }
     }
 }
 
 #ifdef _WIN32
-[[maybe_unused]] auto kill_pcsx2() -> void { (void)system("tskill PCSX2"); }
+[[maybe_unused]] auto kill_pcsx2() -> void {
+    if (system("tskill PCSX2") != 0) {
+        printf("Failed to kill PCSX2!\n");
+    }
+}
 #else
-[[maybe_unused]] auto kill_pcsx2() -> void { (void)system("pkill PCSX2"); }
+[[maybe_unused]] auto kill_pcsx2() -> void {
+    if (system("pkill PCSX2") != 0) {
+        printf("Failed to kill PCSX2!\n");
+    }
+}
+
 #endif
 
 SCENARIO("PCSX2 can be interacted with remotely through IPC", "[pcsx2_ipc]") {
@@ -47,16 +59,6 @@ SCENARIO("PCSX2 can be interacted with remotely through IPC", "[pcsx2_ipc]") {
                 ipc->Write<u64>(0x00347D34, 5);
                 REQUIRE(0 == 1);
             } catch (...) {}
-
-            try {
-                ipc->Write<u128>(0x00347D34, 5);
-                REQUIRE(0 == 1);
-            } catch (...) {}
-
-            try {
-                ipc->Read<u128>(0x00347D34);
-                REQUIRE(0 == 1);
-            } catch (...) {}
         }
     }
 
@@ -70,6 +72,7 @@ SCENARIO("PCSX2 can be interacted with remotely through IPC", "[pcsx2_ipc]") {
                 PCSX2Ipc *ipc = new PCSX2Ipc();
 
                 try {
+                    // unknown opcode test
                     char c_cmd[1];
                     // if we ever implement this opcode this command won't fail
                     c_cmd[0] = 0xFE;
@@ -79,6 +82,42 @@ SCENARIO("PCSX2 can be interacted with remotely through IPC", "[pcsx2_ipc]") {
                     REQUIRE(0 == 1);
                 } catch (...) {}
 
+                try {
+                    // write fail test
+                    char c_cmd[1];
+                    c_cmd[0] = 0xFE;
+                    char c_ret[1];
+                    ipc->SendCommand(PCSX2Ipc::IPCBuffer{ INT_MAX, c_cmd },
+                                     PCSX2Ipc::IPCBuffer{ 1, c_ret });
+                    REQUIRE(0 == 1);
+                } catch (...) {}
+
+                try {
+                    ipc->Write<u128>(0x00347D34, 5);
+                    REQUIRE(0 == 1);
+                } catch (...) {}
+
+                try {
+                    ipc->Read<u128>(0x00347D34);
+                    REQUIRE(0 == 1);
+                } catch (...) {}
+                kill_pcsx2();
+            }
+
+            THEN("It returns errors when socket issues happen") {
+                PCSX2Ipc *ipc = new PCSX2Ipc();
+
+                try {
+                    // read fail test
+                    char c_cmd[1];
+                    c_cmd[0] = 0xFE;
+                    ipc->SendCommand(
+                        PCSX2Ipc::IPCBuffer{ 1, c_cmd },
+                        PCSX2Ipc::IPCBuffer{ INT_MAX, (char *)0x00 });
+                    REQUIRE(0 == 1);
+
+                } catch (...) {}
+
                 kill_pcsx2();
             }
         }
@@ -159,7 +198,6 @@ SCENARIO("PCSX2 can be interacted with remotely through IPC", "[pcsx2_ipc]") {
                     REQUIRE(0 == 1);
                 } catch (...) {}
             }
-
             kill_pcsx2();
         }
     }

utils/build-release.sh

@@ -27,7 +27,8 @@ find release -type d -name target -prune -exec rm -rf {} \;
 # and code coverage because why not :D
 meson build -Db_coverage=true
 cd build
-ninja test
+# pcsx2 takes time to start up D:
+meson test --timeout-multiplier=10
 ninja coverage-html
 mkdir -p release/tests
 cp -rf meson-logs/coveragereport/ ../release/tests