fix(command): broken pipe when ignoring output.

include/components/controller.hpp

@@ -18,6 +18,7 @@ POLYBAR_NS
 
 enum class alignment;
 class bar;
+template <output_policy>
 class command;
 class config;
 class connection;
@@ -80,7 +81,7 @@ class controller
   unique_ptr<bar> m_bar;
   unique_ptr<ipc> m_ipc;
   unique_ptr<inotify_watch> m_confwatch;
-  unique_ptr<command> m_command;
+  unique_ptr<command<output_policy::IGNORED>> m_command;
 
   array<unique_ptr<file_descriptor>, 2> m_queuefd{};
 

include/components/types.hpp

@@ -236,4 +236,9 @@ struct event_timer {
   };
 };
 
+enum class output_policy {
+  REDIRECTED,
+  IGNORED,
+};
+
 POLYBAR_NS_END

include/modules/script.hpp

@@ -27,7 +27,7 @@ namespace modules {
 
     mutex_wrapper<function<chrono::duration<double>()>> m_handler;
 
-    unique_ptr<command> m_command;
+    unique_ptr<command<output_policy::REDIRECTED>> m_command;
 
     bool m_tail;
 

include/utils/command.hpp

@@ -4,6 +4,7 @@
 
 #include "common.hpp"
 #include "components/logger.hpp"
+#include "components/types.hpp"
 #include "errors.hpp"
 #include "utils/factory.hpp"
 #include "utils/functional.hpp"
@@ -15,17 +16,22 @@ DEFINE_ERROR(command_error);
 /**
  * Wrapper used to execute command in a subprocess.
  * In-/output streams are opened to enable ipc.
+ * If the command is created using command_util::make_command<output_policy::REDIRECTED>, the child streams are
+ * redirected and you can read the program output or write into the program input.
+ *
+ * If the command is created using command_util::make_command<output_policy::IGNORED>, the output is not redirected and
+ * you can't communicate with the child program.
  *
  * Example usage:
  *
  * \code cpp
- *   auto cmd = command_util::make_command("cat /etc/rc.local");
+ *   auto cmd = command_util::make_command<output_policy::REDIRECTED>("cat /etc/rc.local");
  *   cmd->exec();
  *   cmd->tail([](string s) { std::cout << s << std::endl; });
  * \endcode
  *
  * \code cpp
- *   auto cmd = command_util::make_command(
+ *   auto cmd = command_util::make_command<output_policy::REDIRECTED>(
  *    "while read -r line; do echo data from parent process: $line; done");
  *   cmd->exec(false);
  *   cmd->writeline("Test");
@@ -34,29 +40,34 @@ DEFINE_ERROR(command_error);
  * \endcode
  *
  * \code cpp
- *   auto cmd = command_util::make_command("for i in 1 2 3; do echo $i; done");
+ *   auto cmd = command_util::make_command<output_policy::REDIRECTED>("for i in 1 2 3; do echo $i; done");
  *   cmd->exec();
  *   cout << cmd->readline(); // 1
  *   cout << cmd->readline() << cmd->readline(); // 23
  * \endcode
+ *
+ * \code cpp
+ *   auto cmd = command_util::make_command<output_policy::IGNORED>("ping kernel.org");
+ *   int status = cmd->exec();
+ * \endcode
  */
-class command {
+template <output_policy>
+class command;
+
+template <>
+class command<output_policy::IGNORED> {
  public:
   explicit command(const logger& logger, string cmd);
-
+  command(const command&) = delete;
   ~command();
 
+  command& operator=(const command&) = delete;
+
   int exec(bool wait_for_completion = true);
   void terminate();
   bool is_running();
   int wait();
 
-  void tail(callback<string> cb);
-  int writeline(string data);
-  string readline();
-
-  int get_stdout(int c);
-  int get_stdin(int c);
   pid_t get_pid();
   int get_exit_status();
 
@@ -65,20 +76,46 @@ class command {
 
   string m_cmd;
 
-  int m_stdout[2]{};
-  int m_stdin[2]{};
-
   pid_t m_forkpid{};
   int m_forkstatus{};
+};
+
+template <>
+class command<output_policy::REDIRECTED> : private command<output_policy::IGNORED> {
+ public:
+  explicit command(const logger& logger, string cmd);
+  command(const command&) = delete;
+  ~command();
+
+  command& operator=(const command&) = delete;
+
+  int exec(bool wait_for_completion = true);
+  using command<output_policy::IGNORED>::terminate;
+  using command<output_policy::IGNORED>::is_running;
+  using command<output_policy::IGNORED>::wait;
+
+  using command<output_policy::IGNORED>::get_pid;
+  using command<output_policy::IGNORED>::get_exit_status;
+
+  void tail(callback<string> cb);
+  int writeline(string data);
+  string readline();
+
+  int get_stdout(int c);
+  int get_stdin(int c);
+
+ protected:
+  int m_stdout[2]{};
+  int m_stdin[2]{};
 
   std::mutex m_pipelock{};
 };
 
 namespace command_util {
-  template <typename... Args>
+  template <output_policy OutputType, typename... Args>
-  unique_ptr<command> make_command(Args&&... args) {
+  unique_ptr<command<OutputType>> make_command(Args&&... args) {
-    return factory_util::unique<command>(logger::make(), forward<Args>(args)...);
+    return factory_util::unique<command<OutputType>>(logger::make(), forward<Args>(args)...);
   }
-}
+}  // namespace command_util
 
 POLYBAR_NS_END

src/adapters/net.cpp

@@ -129,7 +129,7 @@ namespace net {
   bool network::ping() const {
     try {
       auto exec = "ping -c 2 -W 2 -I " + m_interface + " " + string(CONNECTION_TEST_IP);
-      auto ping = command_util::make_command(exec);
+      auto ping = command_util::make_command<output_policy::IGNORED>(exec);
       return ping && ping->exec(true) == EXIT_SUCCESS;
     } catch (const std::exception& err) {
       return false;

src/components/controller.cpp

@@ -408,7 +408,7 @@ void controller::process_inputdata() {
       }
 
       m_log.info("Executing shell command: %s", cmd);
-      m_command = command_util::make_command(move(cmd));
+      m_command = command_util::make_command<output_policy::IGNORED>(move(cmd));
       m_command->exec();
       m_command.reset();
       process_update(true);

src/modules/ipc.cpp

@@ -60,7 +60,7 @@ namespace modules {
    */
   void ipc_module::start() {
     if (m_initial) {
-      auto command = command_util::make_command(m_hooks.at(m_initial - 1)->command);
+      auto command = command_util::make_command<output_policy::REDIRECTED>(m_hooks.at(m_initial - 1)->command);
       command->exec(false);
       command->tail([this](string line) { m_output = line; });
     }
@@ -114,7 +114,7 @@ namespace modules {
       try {
         // Clear the output in case the command produces no output
         m_output.clear();
-        auto command = command_util::make_command(hook->command);
+        auto command = command_util::make_command<output_policy::REDIRECTED>(hook->command);
         command->exec(false);
         command->tail([this](string line) { m_output = line; });
       } catch (const exception& err) {

src/modules/script.cpp

@@ -22,7 +22,7 @@ namespace modules {
             if (!m_command || !m_command->is_running()) {
               string exec{string_util::replace_all(m_exec, "%counter%", to_string(++m_counter))};
               m_log.info("%s: Invoking shell command: \"%s\"", name(), exec);
-              m_command = command_util::make_command(exec);
+              m_command = command_util::make_command<output_policy::REDIRECTED>(exec);
 
               try {
                 m_command->exec(false);
@@ -59,7 +59,7 @@ namespace modules {
           try {
             auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter));
             m_log.info("%s: Invoking shell command: \"%s\"", name(), exec);
-            m_command = command_util::make_command(exec);
+            m_command = command_util::make_command<output_policy::REDIRECTED>(exec);
             m_command->exec(true);
           } catch (const exception& err) {
             m_log.err("%s: %s", name(), err.what());
@@ -143,7 +143,7 @@ namespace modules {
   bool script_module::check_condition() {
     if (m_exec_if.empty()) {
       return true;
-    } else if (command_util::make_command(m_exec_if)->exec(true) == 0) {
+    } else if (command_util::make_command<output_policy::IGNORED>(m_exec_if)->exec(true) == 0) {
       return true;
     } else if (!m_output.empty()) {
       broadcast();

src/utils/command.cpp

@@ -3,6 +3,7 @@
 #include <csignal>
 #include <cstdlib>
 #include <utility>
+#include <utils/string.hpp>
 
 #include "errors.hpp"
 #include "utils/command.hpp"
@@ -18,7 +19,95 @@
 
 POLYBAR_NS
 
-command::command(const logger& logger, string cmd) : m_log(logger), m_cmd(move(cmd)) {
+command<output_policy::IGNORED>::command(const logger& logger, string cmd) : m_log(logger), m_cmd(move(cmd)) {}
+
+command<output_policy::IGNORED>::~command() {
+  if (is_running()) {
+    terminate();
+  }
+}
+
+/**
+ * Execute the command
+ */
+int command<output_policy::IGNORED>::exec(bool wait_for_completion) {
+  if ((m_forkpid = fork()) == -1) {
+    throw system_error("Failed to fork process");
+  }
+
+  if (process_util::in_forked_process(m_forkpid)) {
+    setpgid(m_forkpid, 0);
+    process_util::exec_sh(m_cmd.c_str());
+  } else {
+    if (wait_for_completion) {
+      auto status = wait();
+      m_forkpid = -1;
+      return status;
+    }
+  }
+
+  return EXIT_SUCCESS;
+}
+
+void command<output_policy::IGNORED>::terminate() {
+  if (is_running()) {
+    m_log.trace("command: Sending SIGTERM to running child process (%d)", m_forkpid);
+    killpg(m_forkpid, SIGTERM);
+    wait();
+  }
+  m_forkpid = -1;
+}
+
+/**
+ * Check if command is running
+ */
+bool command<output_policy::IGNORED>::is_running() {
+  return m_forkpid > 0 && process_util::wait_for_completion_nohang(m_forkpid, &m_forkstatus) > -1;
+}
+
+/**
+ * Wait for the child processs to finish
+ */
+int command<output_policy::IGNORED>::wait() {
+  do {
+    m_log.trace("command: Waiting for pid %d to finish...", m_forkpid);
+
+    process_util::wait_for_completion(m_forkpid, &m_forkstatus, WCONTINUED | WUNTRACED);
+
+    if (WIFEXITED(m_forkstatus) && m_forkstatus > 0) {
+      m_log.trace("command: Exited with failed status %d", WEXITSTATUS(m_forkstatus));
+    } else if (WIFEXITED(m_forkstatus)) {
+      m_log.trace("command: Exited with status %d", WEXITSTATUS(m_forkstatus));
+    } else if (WIFSIGNALED(m_forkstatus)) {
+      m_log.trace("command: killed by signal %d", WTERMSIG(m_forkstatus));
+    } else if (WIFSTOPPED(m_forkstatus)) {
+      m_log.trace("command: Stopped by signal %d", WSTOPSIG(m_forkstatus));
+    } else if (WIFCONTINUED(m_forkstatus)) {
+      m_log.trace("command: Continued");
+    } else {
+      break;
+    }
+  } while (!WIFEXITED(m_forkstatus) && !WIFSIGNALED(m_forkstatus));
+
+  return m_forkstatus;
+}
+
+/**
+ * Get command pid
+ */
+pid_t command<output_policy::IGNORED>::get_pid() {
+  return m_forkpid;
+}
+
+/**
+ * Get command exit status
+ */
+int command<output_policy::IGNORED>::get_exit_status() {
+  return m_forkstatus;
+}
+
+command<output_policy::REDIRECTED>::command(const polybar::logger& logger, std::string cmd)
+    : command<output_policy::IGNORED>(logger, move(cmd)) {
   if (pipe(m_stdin) != 0) {
     throw command_error("Failed to allocate input stream");
   }
@@ -27,10 +116,7 @@ command::command(const logger& logger, string cmd) : m_log(logger), m_cmd(move(c
   }
 }
 
-command::~command() {
+command<output_policy::REDIRECTED>::~command() {
-  if (is_running()) {
-    terminate();
-  }
   if (m_stdin[PIPE_READ] > 0) {
     close(m_stdin[PIPE_READ]);
   }
@@ -48,7 +134,7 @@ command::~command() {
 /**
  * Execute the command
  */
-int command::exec(bool wait_for_completion) {
+int command<output_policy::REDIRECTED>::exec(bool wait_for_completion) {
   if ((m_forkpid = fork()) == -1) {
     throw system_error("Failed to fork process");
   }
@@ -99,71 +185,28 @@ int command::exec(bool wait_for_completion) {
   return EXIT_SUCCESS;
 }
 
-void command::terminate() {
-  if (is_running()) {
-    m_log.trace("command: Sending SIGTERM to running child process (%d)", m_forkpid);
-    killpg(m_forkpid, SIGTERM);
-    wait();
-  }
-  m_forkpid = -1;
-}
-
-/**
- * Check if command is running
- */
-bool command::is_running() {
-  return m_forkpid > 0 && process_util::wait_for_completion_nohang(m_forkpid, &m_forkstatus) > -1;
-}
-
-/**
- * Wait for the child processs to finish
- */
-int command::wait() {
-  do {
-    m_log.trace("command: Waiting for pid %d to finish...", m_forkpid);
-
-    process_util::wait_for_completion(m_forkpid, &m_forkstatus, WCONTINUED | WUNTRACED);
-
-    if (WIFEXITED(m_forkstatus) && m_forkstatus > 0) {
-      m_log.trace("command: Exited with failed status %d", WEXITSTATUS(m_forkstatus));
-    } else if (WIFEXITED(m_forkstatus)) {
-      m_log.trace("command: Exited with status %d", WEXITSTATUS(m_forkstatus));
-    } else if (WIFSIGNALED(m_forkstatus)) {
-      m_log.trace("command: killed by signal %d", WTERMSIG(m_forkstatus));
-    } else if (WIFSTOPPED(m_forkstatus)) {
-      m_log.trace("command: Stopped by signal %d", WSTOPSIG(m_forkstatus));
-    } else if (WIFCONTINUED(m_forkstatus)) {
-      m_log.trace("command: Continued");
-    } else {
-      break;
-    }
-  } while (!WIFEXITED(m_forkstatus) && !WIFSIGNALED(m_forkstatus));
-
-  return m_forkstatus;
-}
-
 /**
  * Tail command output
  *
  * \note: This is a blocking call and will not
  * end until the stream is closed
  */
-void command::tail(callback<string> cb) {
+void command<output_policy::REDIRECTED>::tail(callback<string> cb) {
-  io_util::tail(m_stdout[PIPE_READ], move(cb));
+  io_util::tail(m_stdout[PIPE_READ], cb);
 }
 
 /**
  * Write line to command input channel
  */
-int command::writeline(string data) {
+int command<output_policy::REDIRECTED>::writeline(string data) {
   std::lock_guard<std::mutex> lck(m_pipelock);
-  return io_util::writeline(m_stdin[PIPE_WRITE], move(data));
+  return static_cast<int>(io_util::writeline(m_stdin[PIPE_WRITE], data));
 }
 
 /**
  * Read a line from the commands output stream
  */
-string command::readline() {
+string command<output_policy::REDIRECTED>::readline() {
   std::lock_guard<std::mutex> lck(m_pipelock);
   return io_util::readline(m_stdout[PIPE_READ]);
 }
@@ -171,29 +214,15 @@ string command::readline() {
 /**
  * Get command output channel
  */
-int command::get_stdout(int c) {
+int command<output_policy::REDIRECTED>::get_stdout(int c) {
   return m_stdout[c];
 }
 
 /**
  * Get command input channel
  */
-int command::get_stdin(int c) {
+int command<output_policy::REDIRECTED>::get_stdin(int c) {
   return m_stdin[c];
 }
 
-/**
- * Get command pid
- */
-pid_t command::get_pid() {
-  return m_forkpid;
-}
-
-/**
- * Get command exit status
- */
-int command::get_exit_status() {
-  return m_forkstatus;
-}
-
 POLYBAR_NS_END