address some clang tidy complaints in tests

Author: CamJN

test/.clang-tidy

@@ -1,2 +1,2 @@
-Checks: '*,-altera-*,-fuchsia-*,-llvmlibc-*-namespace,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-llvmlibc-restrict-system-libc-headers,-portability-restrict-system-includes,-cppcoreguidelines-pro-type-cstyle-cast,-cppcoreguidelines-pro-type-vararg,-cppcoreguidelines-avoid-magic-numbers,-hicpp-vararg,-readability-magic-numbers,-concurrency-mt-unsafe,-hicpp-no-array-decay,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-avoid-do-while,-cppcoreguidelines-avoid-non-const-global-variables,-misc-use-anonymous-namespace,-misc-const-correctness,-readability-identifier-length,-cppcoreguidelines-pro-type-member-init,-readability-function-cognitive-complexity,-cert-err58-cpp,-hicpp-member-init-readability-simplify-boolean-expr,-modernize-use-trailing-return-type'
+Checks: '*,-altera-*,-fuchsia-*,-llvmlibc-*-namespace,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-llvmlibc-restrict-system-libc-headers,-portability-restrict-system-includes,-cppcoreguidelines-pro-type-cstyle-cast,-cppcoreguidelines-pro-type-vararg,-cppcoreguidelines-avoid-magic-numbers,-hicpp-vararg,-readability-magic-numbers,-concurrency-mt-unsafe,-hicpp-no-array-decay,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-avoid-do-while,-cppcoreguidelines-avoid-non-const-global-variables,-misc-use-anonymous-namespace,-misc-const-correctness,-readability-identifier-length,-cppcoreguidelines-pro-type-member-init,-readability-function-cognitive-complexity,-cert-err58-cpp,-hicpp-member-init-readability-simplify-boolean-expr,-modernize-use-trailing-return-type,-hicpp-member-init,-cert-msc50-cpp,-cert-msc30-c'
 HeaderFilterRegex: 'lib_unit_tests.hpp'

test/src/lib_unit_tests.cpp

@@ -99,45 +99,39 @@ auto inner_strdup(const char* str, size_t size) -> char* {
   return ptr;
 }
 
+struct Child {
+  pid_t pid;
+  explicit Child(pid_t pid): pid(pid){}
+  Child(const Child& other) = delete;
+  Child(const Child&& other) = delete;
+  Child& operator=(Child&& other) = delete;
+  Child& operator=(Child& other) = delete;
+  ~Child() {
+    kill(this->pid, SIGTERM); // signal to child to be done
+  }
+};
 /*
   This is a bit irresponsible as there's no checking but this is only for
   testing, you have to pass the path to the executable, and then all the
   arguments, argv[0] (usually but not always the name of the executable), all
   arguments must be strings
  */
-auto varargs_spawn(const char* executable, ...) -> pid_t {
-  va_list      val;
-  const char** args = nullptr;
-  int          argc = 1; // room for terminating NULL
-
+template <typename... Ts>
+auto spawn(const char* executable, Ts... args) -> Child {
   // Determine number of variadic arguments
-  va_start(val, executable);
-  while (va_arg(val, const char*) != nullptr) {
-    argc++;
-  }
-  va_end(val);
-  // cr_log_info("%d args to spawn:\n", argc);
-  //  Allocate args, put references to command / variadic arguments + NULL in
-  //  args because last val is NULL, and we malloc'd an extra pointer, we're
-  //  good
-  args = static_cast<const char**>(malloc(argc * sizeof(char*)));
-  va_start(val, executable);
-  for (int i = 0; i < argc; i++) {
-    args[i] = va_arg(val, const char*);
-    // cr_log_info("%s\n", args[i]);
-  }
-  va_end(val);
+  const uint size = sizeof...(args) + 1;
+  std::array<const char*, size> array = {args..., NULL};
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored \
   "-Wincompatible-pointer-types-discards-qualifiers"
-return array_spawn(executable, const_cast<char* const*>(args));
+return array_spawn(executable, const_cast<char* const*>(array.data()));
 #pragma clang diagnostic pop
 
 _Exit(EXIT_FAILURE);
 }
 
-auto array_spawn(const char* executable, char* const* argv) -> pid_t {
+auto array_spawn(const char* executable, char* const* argv) -> Child {
   sigset_t emptymask = 0;
   sigemptyset(&emptymask);
   struct sigaction act {};
@@ -151,7 +145,7 @@ auto array_spawn(const char* executable, char* const* argv) -> pid_t {
   if (pid != 0) {
     sigsuspend(&emptymask); // wait for child to be ready
     cr_assert_neq(pid, -1);
-    return pid;
+    return Child(pid);
   }
 
 #pragma clang diagnostic push
@@ -163,20 +157,6 @@ perror("child");
 _Exit(EXIT_FAILURE);
 }
 
-auto get_stdout() -> std::string {
-  (void)fflush(stdout);
-  FILE*      f_stdout = cr_get_redirected_stdout();
-  auto const size     = static_cast<const size_t>(1024 * 8);
-  char       buffer[size];
-  char*      head = buffer;
-  size_t     read = 0;
-  do {
-    read = fread(head, 1, size - (head - buffer), f_stdout);
-    head += read;
-  } while (read > 0 && (head - buffer) < size);
-  return { buffer, static_cast<size_t>(head - buffer) };
-}
-
 ParameterizedTestParameters(argv_argc, successful) {
   const size_t nb_params = 7;
 
@@ -215,30 +195,28 @@ ParameterizedTestParameters(argv_argc, successful) {
 }
 
 ParameterizedTest(test_case* param, argv_argc, successful) {
-  cleanup(kill_pid) pid_t const pid =
-    array_spawn("bin/child", (char* const*)param->argv);
+  Child const child =
+    array_spawn("bin/child", param->argv.data());
 
   cr_assert_no_throw(
                      {
-                       const Getargv::ArgvArgc results(pid);
+                       const Getargv::ArgvArgc results(child.pid);
                        cr_assert_eq(results.size(), param->argc);
                        cr_assert_eq(results.empty(), param->argc == 0);
                        for (int i = 0; i < param->argc; i++) {
-                         cr_expect_str_eq(results[i], param->argv[i], "#%d: actual='%s' expected='%s'", param->argc, results[i], param->argv[i]);
-
+                         cr_expect_str_eq(results[i], param->argv.at(i), "#%d: actual='%s' expected='%s'", param->argc, results[i], param->argv.at(i));
                          const int         index    = (i + 1) * -1;
                          const std::string actual   = results[index];
-                         const std::string expected = param->argv[(int)param->argc + index];
+                         const std::string expected = param->argv.at((int)param->argc + index);
                          cr_expect_eq(actual, expected, "actual='%s' expected='%s'", actual.c_str(), expected.c_str());
                        }
                        size_t index = 0;
                        for (auto arg : results) {
-                         cr_expect_str_eq(arg, param->argv[index], "#%d: actual='%s' expected='%s'", param->argc, arg, param->argv[index]);
+                         cr_expect_str_eq(arg, param->argv.at(index), "#%d: actual='%s' expected='%s'", param->argc, arg, param->argv.at(index));
                          index++;
                        }
                      },
                      std::system_error);
-  kill(pid, SIGTERM); // signal to child to be done
 }
 
 ParameterizedTestParameters(print_argv_of_pid, successful) {
@@ -276,28 +254,30 @@ ParameterizedTestParameters(print_argv_of_pid, successful) {
 }
 
 ParameterizedTest(test_case* param, print_argv_of_pid, successful, .init = cr_redirect_stdout) {
-  cleanup(kill_pid) pid_t const pid =
-    array_spawn("bin/child", (char* const*)param->argv);
+  Child const child =
+    array_spawn("bin/child", param->argv.data());
+
+  cr_assert_no_throw(Getargv::Argv(child.pid).print(), std::system_error);
 
-  cr_assert_no_throw(Getargv::Argv(pid).print(), std::system_error);
-  std::string const actual = get_stdout();
   std::string       expected;
   for (int i = 0; i < param->argc; i++) {
-    const auto* arg = param->argv[i];
+    const auto* arg = param->argv.at(i);
     expected += arg;
     expected += "\0"s;
   }
 
-  cr_assert_eq(actual, expected, "actual: '%.*s'[%ld] != expected: '%.*s'[%ld]", (int)actual.size(), actual.c_str(), actual.size(), (int)expected.size(), expected.c_str(), expected.size());
+  cr_assert_stdout_eq_str(expected.c_str());
 }
 
 Test(print_argv_of_pid, failure) {
   std::string const             empty;
   const char*                   argv[] = { empty.c_str(), nullptr };
-  cleanup(kill_pid) pid_t const pid    = array_spawn("bin/child", (char* const*)argv);
+  Child const child = array_spawn("bin/child", (char* const*)argv);
 
+// NOLINTBEGIN(cppcoreguidelines-owning-memory)
   (void)fclose(stdout);
-  cr_assert_throw(Getargv::Argv(pid).print(), std::system_error);
+// NOLINTEND(cppcoreguidelines-owning-memory)
+  cr_assert_throw(Getargv::Argv(child.pid).print(), std::system_error);
 }
 
 ParameterizedTestParameters(argv_of_pid_empty, correct) {
@@ -319,18 +299,17 @@ ParameterizedTestParameters(argv_of_pid_empty, correct) {
 }
 
 ParameterizedTest(test_case* param, argv_of_pid_empty, correct) {
-  cleanup(kill_pid) pid_t const pid =
-    array_spawn("bin/child", (char* const*)param->argv);
+  Child const child = array_spawn("bin/child", param->argv.data());
 
-  cr_assert_eq(Getargv::Argv::as_bytes(pid).empty(), param->argc == 0, "empty() was wrong, should be %s", param->argc == 0 ? "true" : "false");
+  cr_assert_eq(Getargv::Argv::as_bytes(child.pid).empty(), param->argc == 0, "empty() was wrong, should be %s", param->argc == 0 ? "true" : "false");
 }
 
 Test(argv_of_pid_indexing, works) {
-  const std::string             expected = "abcdefghijklmnopqrstuvwxyz";
-  const char*                   argv[]   = { expected.c_str(), nullptr };
-  cleanup(kill_pid) pid_t const pid      = array_spawn("bin/child", (char* const*)argv);
+  const std::string expected = "abcdefghijklmnopqrstuvwxyz";
+  const char* argv[] = { expected.c_str(), nullptr };
+  Child const child  = array_spawn("bin/child", (char* const*)argv);
 
-  const Getargv::Argv args(pid);
+  const Getargv::Argv args(child.pid);
   for (int i = 0; i < expected.size(); i++) {
     cr_assert_eq(args[i], argv[0][i]);
   }
@@ -340,45 +319,45 @@ Test(argv_of_pid_indexing, works) {
 }
 
 Test(argv_of_pid_indexing, failure) {
-  const char*                   argv[] = { "", nullptr };
-  cleanup(kill_pid) pid_t const pid    = array_spawn("bin/child", (char* const*)argv);
+  const char* argv[] = { "", nullptr };
+  Child const child  = array_spawn("bin/child", (char* const*)argv);
 
-  const Getargv::Argv args(pid);
+  const Getargv::Argv args(child.pid);
   cr_assert_throw(args[100000], std::out_of_range);
 }
 Test(argv_argc_of_pid_indexing, failure) {
-  const char*                   argv[] = { "", nullptr };
-  cleanup(kill_pid) pid_t const pid    = array_spawn("bin/child", (char* const*)argv);
+  const char* argv[] = { "", nullptr };
+  Child const child  = array_spawn("bin/child", (char* const*)argv);
 
-  const Getargv::ArgvArgc args(pid);
+  const Getargv::ArgvArgc args(child.pid);
   cr_assert_throw(args[100000], std::out_of_range);
 }
 
 Test(argv_as_string, works) {
-  const std::string             expected = "abcdefghijklmnopqrstuvwxyz";
-  const char*                   argv[]   = { expected.c_str(), nullptr };
-  cleanup(kill_pid) pid_t const pid      = array_spawn("bin/child", (char* const*)argv);
+  const std::string expected = "abcdefghijklmnopqrstuvwxyz";
+  const char* argv[] = { expected.c_str(), nullptr };
+  Child const child  = array_spawn("bin/child", (char* const*)argv);
 
-  const std::string actual = Getargv::Argv::as_string(pid);
+  const std::string actual = Getargv::Argv::as_string(child.pid);
   cr_assert_eq(actual, expected);
 }
 
 Test(argv_argc, to_string_array) {
-  cleanup(kill_pid) pid_t const pid = spawn("bin/child", "bin/child");
+  Child child = spawn("bin/child", "bin/child");
 
   cr_assert_no_throw(
                      {
-                       const Getargv::ArgvArgc        results(pid);
+                       const Getargv::ArgvArgc        results(child.pid);
                        const std::vector<std::string> array = results.to_string_array();
                      },
                      std::system_error,
                      "error thrown");
 }
 
 Test(argv_argc, as_string_array) {
-  cleanup(kill_pid) pid_t const pid = spawn("bin/child", "bin/child");
+  Child const child = spawn("bin/child", "bin/child");
 
-  cr_assert_no_throw(Getargv::ArgvArgc::as_string_array(pid), std::system_error, "error thrown");
+  cr_assert_no_throw(Getargv::ArgvArgc::as_string_array(child.pid), std::system_error, "error thrown");
 }
 
 Test(argv, convert_from_ffi_type) {
@@ -409,54 +388,47 @@ Test(argv_argc, not_copyable) {
 
 Test(argv, simple) {
   const std::string             expected = "bin/child\0"s;
-  cleanup(kill_pid) pid_t const pid      = spawn(expected.c_str(), expected.c_str());
+  Child const child      = spawn(expected.c_str(), expected.c_str());
 
   cr_expect_no_throw(
                      {
-                       const Getargv::Argv proc_ptrs(pid, 0, true);
+                       const Getargv::Argv proc_ptrs(child.pid, 0, true);
                        cr_assert_eq(proc_ptrs.size(), expected.size());
                        const std::string actual(proc_ptrs.begin(), proc_ptrs.end());
                        cr_assert_eq(actual, expected);
                      },
                      std::system_error,
                      "Argv constructor threw an exception when it shouldn't have");
-
-  kill(pid, SIGTERM); // signal to child to be done
 }
 
 Test(argv, nuls_false) {
   const std::string expected = "one\0two\0three\0"s;
 
-  cleanup(kill_pid) pid_t const pid = spawn("bin/child", "one", "two", "three");
+  Child child = spawn("bin/child", "one", "two", "three");
 
   cr_expect_no_throw(
                      {
-                       const Getargv::Argv proc_ptrs(pid, 0, false);
+                       const Getargv::Argv proc_ptrs(child.pid, 0, false);
                        cr_assert_eq(proc_ptrs.size(), expected.size());
                        const std::string actual(proc_ptrs.begin(), proc_ptrs.end());
                        cr_assert_eq(actual, expected);
                      },
                      std::system_error,
                      "Argv constructor threw an exception when it shouldn't have");
-
-  kill(pid, SIGTERM); // signal to child to be done
 }
 
 Test(argv, nuls_true) {
   const std::string expected = "bin/tests --verbose 2 -j1\0"s;
-
-  cleanup(kill_pid) pid_t const pid =
-    spawn("bin/child", "bin/tests", "--verbose", "2", "-j1");
+  Child child = spawn("bin/child", "bin/tests", "--verbose", "2", "-j1");
 
   cr_expect_no_throw(
                      {
-                       const Getargv::Argv proc_ptrs(pid, 0, true);
+                       const Getargv::Argv proc_ptrs(child.pid, 0, true);
                        cr_assert_eq(proc_ptrs.size(), expected.size());
                        const std::string actual(proc_ptrs.begin(), proc_ptrs.end());
                        cr_assert_eq(actual, expected);
                      },
                      std::system_error);
-  kill(pid, SIGTERM); // signal to child to be done
 }
 
 Test(argv, skip_one) {
@@ -519,9 +491,6 @@ void free_strings(struct criterion_test_params* crp) {
 
 void free_argv_argc_test_case(struct criterion_test_params* crp) {
   auto* params = static_cast<test_case*>(crp->params);
-  for (size_t i = 0; i < crp->length; ++i) {
-    cr_free(params[i].argv);
-  }
   cr_free(params);
 }
 
@@ -531,7 +500,3 @@ void initialize_argv_argc_test_case(test_case* ptr) {
     index = nullptr;
   }
 }
-
-void kill_pid(const pid_t* pid) {
-  kill(*pid, SIGTERM); // signal to child to be done
-}

test/src/lib_unit_tests.hpp

@@ -7,8 +7,8 @@ namespace Getargv::ffi {
   using errno_t = ::errno_t;
 } // namespace Getargv::ffi
 
-#include "../../src/argv.cpp"
-#include "../../src/argvargc.cpp"
+#include "../../src/argv.cpp" // NOLINT(bugprone-suspicious-include)
+#include "../../src/argvargc.cpp" // NOLINT(bugprone-suspicious-include)
 
 #include <criterion/criterion.h>
 #include <criterion/hooks.h>
@@ -24,23 +24,24 @@ namespace Getargv::ffi {
 #include <cstdio>
 #include <cstdlib>
 
+struct Child;
+
 auto numPlaces(int n) -> int;
 auto randUpTo(int n) -> int;
 void redirect_all_std();
 auto inner_strdup(const char* str, size_t size) -> char*;
 auto read_file(FILE* file) -> std::string;
-auto inner_spawn(const char* executable, ...) -> pid_t;
+auto inner_spawn(const char* executable, ...) -> Child;
 void sig_handler(int sig);
-auto array_spawn(const char* executable, char* const* argv) -> pid_t;
-auto varargs_spawn(const char* executable, ...) -> pid_t;
+auto array_spawn(const char* executable, char* const* argv) -> Child;
 
-#define cr_strdup(str_arg) inner_strdup(str_arg, sizeof(str_arg))
-#define spawn(...)         varargs_spawn(__VA_ARGS__, NULL)
-#define cleanup(callback)  __attribute__((__cleanup__(callback)))
+constexpr char* cr_strdup(const char* str_arg) {
+  return inner_strdup(str_arg, sizeof(str_arg));
+}
 
 struct get_argv_and_argc_of_pid_test_case {
   uint        argc;
-  const char* argv[5];
+  std::array<char*,5> argv;
 };
 using test_case = struct get_argv_and_argc_of_pid_test_case;