From 86dcd6f92db5bc63b062500c8bb03d8e96e4d306 Mon Sep 17 00:00:00 2001
From: oxmox <oxmox@noreply.localhost>
Date: Sat, 18 Feb 2023 22:28:15 +0100
Subject: [PATCH] get rid of the doomId concept, use the pid instead

The internal unique id should not be needed, the pid is unique enough :)

Also this can allow starting dooms externally without having to pass an id
parameter that's not already in use. The externally started doom sends its
pid with the DoomReady message and the controller can add a new entry if it
needs to. Of course the controller will not be able to kill() these
externally started dooms.
---
 src/doompanning.cc | 29 +++++++++--------------------
 src/doomsim.cc     | 24 ++++--------------------
 src/dp_common.h    |  3 ++-
 3 files changed, 15 insertions(+), 41 deletions(-)

diff --git a/src/doompanning.cc b/src/doompanning.cc
index 52ec08f..ba5615b 100644
--- a/src/doompanning.cc
+++ b/src/doompanning.cc
@@ -27,7 +27,6 @@ void dp_sdl_fatal(const char *const msg)
 struct DoomState
 {
     doomid_t id;
-    pid_t pid;
     DP_DoomState state = DP_DS_Unknown;
 };
 
@@ -41,7 +40,6 @@ struct ControllerContext
     SDL_Window *window;
     SDL_Renderer *renderer;
     std::vector<DoomState> dooms;
-    doomid_t nextDoomid = 0;
     bool quit = false;
     ExampleAppLog appLog;
     int columns = 4;
@@ -56,22 +54,12 @@ struct ControllerActions
 void spawn_doom(ControllerContext &ctx)
 {
     DoomState ds = {};
-    ds.id = ctx.nextDoomid;
 
-    std::array<char, 32> idBuf;
-
-    const char *const argv[] =
-    {
-        "doomsim",
-        "-doomid",
-        aprintf(idBuf, "%zu", ds.id),
-        nullptr
-    };
+    const char *const argv[] = { "doomsim", nullptr };
 
     // TODO: Close stdin and stdout? Leave them open for now to see the logging
     // output.
-    // TODO: any way to get rid of the const_cast?
-    if (auto err = posix_spawn(&ds.pid, "doomsim", nullptr, nullptr,
+    if (auto err = posix_spawn(&ds.id, "doomsim", nullptr, nullptr,
         const_cast<char *const *>(argv), nullptr))
     {
         log_error("Could not spawn doom: %s", strerror(err));
@@ -79,9 +67,8 @@ void spawn_doom(ControllerContext &ctx)
     }
 
     ctx.dooms.emplace_back(ds);
-    ++ctx.nextDoomid;
 
-    log_info("Spawned doom#%zu, pid=%d", ds.id, ds.pid);
+    log_info("Spawned new doom, pid=%d", ds.id);
 }
 
 void end_all_dooms(ControllerContext &ctx)
@@ -105,7 +92,7 @@ void signal_all_dooms(ControllerContext &ctx, int signum)
     std::for_each(std::begin(ctx.dooms), std::end(ctx.dooms),
         [signum] (const auto &ds)
         {
-            kill(ds.pid, signum);
+            kill(ds.id, signum);
         });
 }
 
@@ -135,14 +122,16 @@ void check_on_dooms(ControllerContext &ctx)
 
         if (pid = waitpid(0, &wstatus, WNOHANG); pid > 0)
         {
-            auto dit = find_in_container(ctx.dooms, [pid] (const auto &ds) { return ds.pid == pid; });
+            auto dit = find_in_container(ctx.dooms, [pid] (const auto &ds) { return ds.id == pid; });
+
             assert(dit != std::end(ctx.dooms));
+
             if (dit != std::end(ctx.dooms))
             {
                 if (WIFEXITED(wstatus))
-                    log_info("doom#%zu, pid=%d exited with status %d", dit->id, pid, WEXITSTATUS(wstatus));
+                    log_info("doom(%d) exited with status %d", pid, WEXITSTATUS(wstatus));
                 else if (WIFSIGNALED(wstatus))
-                    log_warn("doom#%zu, pid=%d got killed by signal %d", dit->id, pid, WTERMSIG(wstatus));
+                    log_warn("doom#(%d) got killed by signal %d", pid, WTERMSIG(wstatus));
 
                 ctx.dooms.erase(dit);
             }
diff --git a/src/doomsim.cc b/src/doomsim.cc
index edcfab4..b2e927d 100644
--- a/src/doomsim.cc
+++ b/src/doomsim.cc
@@ -1,7 +1,5 @@
 #include <cassert>
 #include <cstdlib>
-#include <cstring>
-#include <optional>
 #include <nng/nng.h>
 #include <nng/protocol/pubsub0/pub.h>
 #include <nng/protocol/pubsub0/sub.h>
@@ -169,7 +167,7 @@ DEF_DOOM_STATE_FUNC(do_doom_running)
 
 int doom_loop(DoomContext *ctx)
 {
-    log_debug("doom#%zu started", ctx->id);
+    log_debug("doom(%d) started", ctx->id);
 
     int res = 0;
 
@@ -189,6 +187,8 @@ int doom_loop(DoomContext *ctx)
 
 int main(int argc, char *argv[])
 {
+    (void) argc;
+    (void) argv;
 #ifndef NDEBUG
     log_set_level(LOG_TRACE);
 #else
@@ -198,22 +198,6 @@ int main(int argc, char *argv[])
 
     log_info("doomsim starting");
 
-    std::optional<doomid_t> doomId;
-
-    for (int i=1; i<argc; ++i)
-    {
-        if (i < argc-1 && strcmp(argv[i], "-doomid") == 0)
-        {
-            doomId = std::strtoull(argv[i+1], nullptr, 0);
-        }
-    }
-
-    if (!doomId)
-    {
-        log_error("Missing -doomid <id> parameter!");
-        return 1;
-    }
-
     dp_nng_init_limits(1, 1, 1);
 
     auto pubSock = make_doom_pub(DoomUrl);
@@ -223,7 +207,7 @@ int main(int argc, char *argv[])
     {
         pubSock,
         subSock,
-        *doomId,
+        getpid(),
         DP_DS_Ready,
         do_doom_ready,
     };
diff --git a/src/dp_common.h b/src/dp_common.h
index fb3782b..7b20b93 100644
--- a/src/dp_common.h
+++ b/src/dp_common.h
@@ -3,6 +3,7 @@
 
 #include <nng/nng.h>
 #include <stdlib.h>
+#include <sys/types.h>
 #include "dp_types.h"
 #include "log.h"
 
@@ -12,7 +13,7 @@ extern "C" {
 
 #define dp_fatal(...) log_fatal(__VA_ARGS__)
 
-typedef u32 doomid_t; // unique id for each doom instance
+typedef pid_t doomid_t; // unique id for each doom instance
 typedef u16 dmt_t; // for DP_MessageType values
 
 typedef enum DP_MessageType