fixed lots of not-thread-safe things
Most of the things I fixed here were using libc functions that are
fundamentally not thread-safe.
diff --git a/aos/build/aos.gyp b/aos/build/aos.gyp
index 916db03..3aaaf10 100644
--- a/aos/build/aos.gyp
+++ b/aos/build/aos.gyp
@@ -25,10 +25,10 @@
],
'dependencies': [
'<(AOS)/common/common.gyp:die',
- '<(AOS)/common/util/util.gyp:aos_strerror',
+ '<(AOS)/common/libc/libc.gyp:aos_strerror',
],
'export_dependent_settings': [
- '<(AOS)/common/util/util.gyp:aos_strerror',
+ '<(AOS)/common/libc/libc.gyp:aos_strerror',
],
},
{
diff --git a/aos/build/aos_all.gyp b/aos/build/aos_all.gyp
index 8623d51..4f9e4ab 100644
--- a/aos/build/aos_all.gyp
+++ b/aos/build/aos_all.gyp
@@ -10,19 +10,23 @@
'no_rsync': 1,
},
'dependencies': [
- '../linux_code/linux_code.gyp:core',
- '../linux_code/logging/logging.gyp:binary_log_writer',
- '../linux_code/logging/logging.gyp:log_streamer',
- '../linux_code/logging/logging.gyp:log_displayer',
- '../linux_code/ipc_lib/ipc_lib.gyp:raw_queue_test',
- '../linux_code/ipc_lib/ipc_lib.gyp:ipc_stress_test',
- '../linux_code/starter/starter.gyp:starter_exe',
- '../linux_code/starter/starter.gyp:netconsole',
- '../common/common.gyp:queue_test',
- '../common/common.gyp:die_test',
- '../common/common.gyp:queue_types_test',
- '../common/util/util.gyp:trapezoid_profile_test',
- '../common/util/util.gyp:wrapping_counter_test',
+ '<(AOS)/linux_code/linux_code.gyp:core',
+ '<(AOS)/linux_code/logging/logging.gyp:binary_log_writer',
+ '<(AOS)/linux_code/logging/logging.gyp:log_streamer',
+ '<(AOS)/linux_code/logging/logging.gyp:log_displayer',
+ '<(AOS)/linux_code/ipc_lib/ipc_lib.gyp:raw_queue_test',
+ '<(AOS)/linux_code/ipc_lib/ipc_lib.gyp:ipc_stress_test',
+ '<(AOS)/linux_code/starter/starter.gyp:starter_exe',
+ '<(AOS)/linux_code/starter/starter.gyp:netconsole',
+ '<(AOS)/common/common.gyp:queue_test',
+ '<(AOS)/common/common.gyp:die_test',
+ '<(AOS)/common/common.gyp:queue_types_test',
+ '<(AOS)/common/util/util.gyp:trapezoid_profile_test',
+ '<(AOS)/common/util/util.gyp:wrapping_counter_test',
+ '<(AOS)/common/libc/libc.gyp:dirname_test',
+ '<(AOS)/common/libc/libc.gyp:aos_strerror_test',
+ '<(AOS)/common/libc/libc.gyp:aos_strsignal_test',
+ '<(AOS)/common/util/util.gyp:run_command_test',
'<(DEPTH)/bbb_cape/src/bbb/bbb.gyp:cows_test',
'<(DEPTH)/bbb_cape/src/bbb/bbb.gyp:packet_finder_test',
'Common',
diff --git a/aos/common/common.gyp b/aos/common/common.gyp
index 2581eaa..186599a 100644
--- a/aos/common/common.gyp
+++ b/aos/common/common.gyp
@@ -226,10 +226,10 @@
'die.cc',
],
'dependencies': [
- '<(AOS)/common/util/util.gyp:aos_strerror',
+ '<(AOS)/common/libc/libc.gyp:aos_strerror',
],
'export_dependent_settings': [
- '<(AOS)/common/util/util.gyp:aos_strerror',
+ '<(AOS)/common/libc/libc.gyp:aos_strerror',
],
},
{
diff --git a/aos/common/die.h b/aos/common/die.h
index 05500be..80e8450 100644
--- a/aos/common/die.h
+++ b/aos/common/die.h
@@ -4,7 +4,7 @@
#include <stdarg.h>
#include "aos/common/macros.h"
-#include "aos/common/util/aos_strerror.h"
+#include "aos/common/libc/aos_strerror.h"
namespace aos {
diff --git a/aos/common/libc/README b/aos/common/libc/README
new file mode 100644
index 0000000..c4ee817
--- /dev/null
+++ b/aos/common/libc/README
@@ -0,0 +1,18 @@
+This directory has replacements for some libc functions that we don't want to
+use because they're not thread-safe and/or they allocate memory.
+
+Some of them are implemented as C++ functions because it makes fixing the
+memory management issues that cause the standard versions to be not thread-safe
+possible and some of them are still callable from C when it makes sense.
+
+<http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_09_01>
+has a list of the not-thread-safe POSIX functions. We've gotten rid of all of
+those except for the following:
+ getenv(3): <http://austingroupbugs.net/view.php?id=188> proposes changing
+ POSIX to require it to return a pointer into environ, making it thread-safe,
+ which glibc already does.
+ inet_ntoa(3): Glibc uses a thread-local buffer, which makes it thread-safe,
+ and uses of this function should go away soon.
+ readdir(3): Glibc already guarantees that only invocations on the same
+ directory stream aren't thread-safe, and there's talk of changing POSIX to
+ say the same thing.
diff --git a/aos/common/util/aos_strerror.cc b/aos/common/libc/aos_strerror.cc
similarity index 92%
rename from aos/common/util/aos_strerror.cc
rename to aos/common/libc/aos_strerror.cc
index d2b2ca6..d5fc2b4 100644
--- a/aos/common/util/aos_strerror.cc
+++ b/aos/common/libc/aos_strerror.cc
@@ -1,4 +1,4 @@
-#include "aos/common/util/aos_strerror.h"
+#include "aos/common/libc/aos_strerror.h"
#include <assert.h>
#include <sys/types.h>
@@ -33,7 +33,7 @@
} // namespace
-char *aos_strerror(int error) {
+const char *aos_strerror(int error) {
static AOS_THREAD_LOCAL char buffer[kBufferSize];
// Call the overload for whichever version we're using.
diff --git a/aos/common/util/aos_strerror.h b/aos/common/libc/aos_strerror.h
similarity index 67%
rename from aos/common/util/aos_strerror.h
rename to aos/common/libc/aos_strerror.h
index 2fd6818..acce427 100644
--- a/aos/common/util/aos_strerror.h
+++ b/aos/common/libc/aos_strerror.h
@@ -1,5 +1,5 @@
-#ifndef AOS_COMMON_UTIL_AOS_STRERROR_H_
-#define AOS_COMMON_UTIL_AOS_STRERROR_H_
+#ifndef AOS_COMMON_LIBC_AOS_STRERROR_H_
+#define AOS_COMMON_LIBC_AOS_STRERROR_H_
#ifdef __cplusplus
extern "C" {
@@ -7,15 +7,17 @@
// Thread-safe version of strerror(3) (except it may change errno).
//
+// Returns a pointer to static data or a thread-local buffer.
+//
// Necessary because strerror_r(3) is such a mess (which version you get is
// determined at compile time by black magic related to feature macro
// definitions, compiler flags, glibc version, and even whether you're using g++
// or clang++) and strerror_l(3) might not work if you end up with the magic
// LC_GLOBAL_LOCALE locale.
-char *aos_strerror(int error);
+const char *aos_strerror(int error);
#ifdef __cplusplus
}
#endif
-#endif // AOS_COMMON_UTIL_AOS_STRERROR_H_
+#endif // AOS_COMMON_LIBC_AOS_STRERROR_H_
diff --git a/aos/common/libc/aos_strerror_test.cc b/aos/common/libc/aos_strerror_test.cc
new file mode 100644
index 0000000..c4574fe
--- /dev/null
+++ b/aos/common/libc/aos_strerror_test.cc
@@ -0,0 +1,29 @@
+#include "aos/common/libc/aos_strerror.h"
+
+#include <errno.h>
+
+#include "gtest/gtest.h"
+
+namespace aos {
+namespace libc {
+namespace testing {
+
+// Tries a couple of easy ones.
+TEST(StrerrorTest, Basic) {
+ EXPECT_STREQ("Argument list too long", aos_strerror(E2BIG));
+ EXPECT_STREQ("Bad file descriptor", aos_strerror(EBADF));
+ EXPECT_STREQ("Unknown error 4021", aos_strerror(4021));
+}
+
+// Runs through all errno values and makes sure it gives the same result as
+// strerror(3).
+TEST(StrerrorTest, All) {
+ for (int i = 0; i < 4095; ++i) {
+ SCOPED_TRACE("iteration " + ::std::to_string(i));
+ EXPECT_STREQ(strerror(i), aos_strerror(i));
+ }
+}
+
+} // namespace testing
+} // namespace libc
+} // namespace aos
diff --git a/aos/common/libc/aos_strsignal.cc b/aos/common/libc/aos_strsignal.cc
new file mode 100644
index 0000000..5fae327
--- /dev/null
+++ b/aos/common/libc/aos_strsignal.cc
@@ -0,0 +1,23 @@
+#include "aos/common/libc/aos_strsignal.h"
+
+#include <signal.h>
+
+#include "aos/linux_code/thread_local.h"
+#include "aos/common/logging/logging.h"
+
+const char *aos_strsignal(int signal) {
+ static AOS_THREAD_LOCAL char buffer[512];
+
+ if (signal >= SIGRTMIN && signal <= SIGRTMAX) {
+ CHECK(snprintf(buffer, sizeof(buffer), "Real-time signal %d",
+ signal - SIGRTMIN) > 0);
+ return buffer;
+ }
+
+ if (signal > 0 && signal < NSIG && sys_siglist[signal] != nullptr) {
+ return sys_siglist[signal];
+ }
+
+ CHECK(snprintf(buffer, sizeof(buffer), "Unknown signal %d", signal) > 0);
+ return buffer;
+}
diff --git a/aos/common/libc/aos_strsignal.h b/aos/common/libc/aos_strsignal.h
new file mode 100644
index 0000000..ef6795c
--- /dev/null
+++ b/aos/common/libc/aos_strsignal.h
@@ -0,0 +1,17 @@
+#ifndef AOS_COMMON_LIBC_AOS_STRSIGNAL_H_
+#define AOS_COMMON_LIBC_AOS_STRSIGNAL_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// Thread-safe version of strsignal(3) (except it will never return NULL).
+//
+// Returns a pointer to static data or a thread-local buffer.
+const char *aos_strsignal(int signal);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif // AOS_COMMON_LIBC_AOS_STRSIGNAL_H_
diff --git a/aos/common/libc/aos_strsignal_test.cc b/aos/common/libc/aos_strsignal_test.cc
new file mode 100644
index 0000000..50cf635
--- /dev/null
+++ b/aos/common/libc/aos_strsignal_test.cc
@@ -0,0 +1,28 @@
+#include "aos/common/libc/aos_strsignal.h"
+
+#include <signal.h>
+
+#include "gtest/gtest.h"
+
+namespace aos {
+namespace libc {
+namespace testing {
+
+// Tries a couple of easy ones.
+TEST(StrsignalTest, Basic) {
+ EXPECT_STREQ("Hangup", aos_strsignal(SIGHUP));
+ EXPECT_STREQ("Broken pipe", aos_strsignal(SIGPIPE));
+ EXPECT_STREQ("Real-time signal 2", aos_strsignal(SIGRTMIN + 2));
+ EXPECT_STREQ("Unknown signal 155", aos_strsignal(155));
+}
+
+// Tests that all the signals give the same result as strsignal(3).
+TEST(StrsignalTest, All) {
+ for (int i = 0; i < SIGRTMAX + 5; ++i) {
+ EXPECT_STREQ(strsignal(i), aos_strsignal(i));
+ }
+}
+
+} // namespace testing
+} // namespace libc
+} // namespace aos
diff --git a/aos/common/libc/dirname.cc b/aos/common/libc/dirname.cc
new file mode 100644
index 0000000..c843898
--- /dev/null
+++ b/aos/common/libc/dirname.cc
@@ -0,0 +1,39 @@
+#include "aos/common/libc/dirname.h"
+
+namespace aos {
+namespace libc {
+namespace {
+
+::std::string DoDirname(const ::std::string &path, size_t last_slash) {
+ // If there aren't any other '/'s in it.
+ if (last_slash == ::std::string::npos) return ".";
+
+ // Back up as long as we see '/'s.
+ do {
+ // If we get all the way to the beginning.
+ if (last_slash == 0) return "/";
+ --last_slash;
+ } while (path[last_slash] == '/');
+
+ return path.substr(0, last_slash + 1);
+}
+
+} // namespace
+
+::std::string Dirname(const ::std::string &path) {
+ // Without this, we end up with integer underflows below, which is technically
+ // undefined.
+ if (path.size() == 0) return ".";
+
+ size_t last_slash = path.rfind('/');
+
+ // If the path ends with a '/'.
+ if (last_slash == path.size() - 1) {
+ last_slash = DoDirname(path, last_slash).rfind('/');
+ }
+
+ return DoDirname(path, last_slash);
+}
+
+} // namespace libc
+} // namespace aos
diff --git a/aos/common/libc/dirname.h b/aos/common/libc/dirname.h
new file mode 100644
index 0000000..f05f8f1
--- /dev/null
+++ b/aos/common/libc/dirname.h
@@ -0,0 +1,15 @@
+#ifndef AOS_COMMON_LIBC_DIRNAME_H_
+#define AOS_COMMON_LIBC_DIRNAME_H_
+
+#include <string>
+
+namespace aos {
+namespace libc {
+
+// Thread-safe version of dirname(3).
+::std::string Dirname(const ::std::string &path);
+
+} // namespace libc
+} // namespace aos
+
+#endif // AOS_COMMON_LIBC_DIRNAME_H_
diff --git a/aos/common/libc/dirname_test.cc b/aos/common/libc/dirname_test.cc
new file mode 100644
index 0000000..0207961
--- /dev/null
+++ b/aos/common/libc/dirname_test.cc
@@ -0,0 +1,79 @@
+#include "aos/common/libc/dirname.h"
+
+#include <libgen.h>
+
+#include "gtest/gtest.h"
+
+namespace aos {
+namespace libc {
+namespace testing {
+
+// Tests the examples from the Linux man-pages release 3.44 dirname(3).
+TEST(DirnameTest, ManPageExamples) {
+ EXPECT_EQ("/usr", Dirname("/usr/lib"));
+ EXPECT_EQ("/", Dirname("/usr/"));
+ EXPECT_EQ(".", Dirname("usr"));
+ EXPECT_EQ("/", Dirname("/"));
+ EXPECT_EQ(".", Dirname("."));
+ EXPECT_EQ(".", Dirname(".."));
+}
+
+// Tests that it handles multiple '/'s in a row correctly.
+TEST(DirnameTest, MultipleSlashes) {
+ EXPECT_EQ("//usr", Dirname("//usr//lib"));
+ EXPECT_EQ("//usr/lib", Dirname("//usr/lib//bla"));
+ EXPECT_EQ("/", Dirname("//usr//"));
+ EXPECT_EQ(".", Dirname("usr//"));
+ EXPECT_EQ("/", Dirname("//"));
+ EXPECT_EQ(".", Dirname(".//"));
+ EXPECT_EQ(".", Dirname("..//"));
+}
+
+TEST(DirnameTest, WeirdInputs) {
+ EXPECT_EQ(".", Dirname(""));
+ EXPECT_EQ(".", Dirname("..."));
+}
+
+// Runs through a bunch of randomly constructed pathnames and makes sure it
+// gives the same result as dirname(3).
+TEST(DirnameTest, Random) {
+ static const char kTestBytes[] = "a0//.. ";
+ static const size_t kTestBytesSize = sizeof(kTestBytes) - 1;
+ static const size_t kTestPathSize = 6;
+
+ ::std::string test_string(kTestPathSize, '\0');
+ char test_path[kTestPathSize + 1];
+ for (size_t i0 = 0; i0 < kTestBytesSize; ++i0) {
+ test_string[0] = kTestBytes[i0];
+ for (size_t i1 = 0; i1 < kTestBytesSize; ++i1) {
+ // dirname(3) returns "//" in this case which is weird and our Dirname
+ // doesn't.
+ if (test_string[0] == '/' && kTestBytes[i1] == '/') continue;
+
+ test_string[1] = kTestBytes[i1];
+ for (size_t i2 = 0; i2 < kTestBytesSize; ++i2) {
+ test_string[2] = kTestBytes[i2];
+ for (size_t i3 = 0; i3 < kTestBytesSize; ++i3) {
+ test_string[3] = kTestBytes[i3];
+ for (size_t i4 = 0; i4 < kTestBytesSize; ++i4) {
+ test_string[4] = kTestBytes[i4];
+ for (size_t i5 = 0; i5 < kTestBytesSize; ++i5) {
+ test_string[5] = kTestBytes[i5];
+
+ memcpy(test_path, test_string.c_str(), kTestPathSize);
+ test_path[kTestPathSize] = '\0';
+
+ SCOPED_TRACE("path is '" + test_string + "'");
+ EXPECT_EQ(::std::string(dirname(test_path)),
+ Dirname(test_string));
+ }
+ }
+ }
+ }
+ }
+ }
+}
+
+} // namespace testing
+} // namespace libc
+} // namespace aos
diff --git a/aos/common/libc/libc.gyp b/aos/common/libc/libc.gyp
new file mode 100644
index 0000000..d7fc357
--- /dev/null
+++ b/aos/common/libc/libc.gyp
@@ -0,0 +1,62 @@
+{
+ 'targets': [
+ {
+ 'target_name': 'aos_strsignal',
+ 'type': 'static_library',
+ 'sources': [
+ 'aos_strsignal.cc',
+ ],
+ 'dependencies': [
+ '<(AOS)/build/aos.gyp:logging_interface',
+ ],
+ },
+ {
+ 'target_name': 'aos_strsignal_test',
+ 'type': 'executable',
+ 'sources': [
+ 'aos_strsignal_test.cc',
+ ],
+ 'dependencies': [
+ 'aos_strsignal',
+ '<(EXTERNALS):gtest',
+ '<(AOS)/build/aos.gyp:logging',
+ ],
+ },
+ {
+ 'target_name': 'dirname',
+ 'type': 'static_library',
+ 'sources': [
+ 'dirname.cc',
+ ],
+ },
+ {
+ 'target_name': 'dirname_test',
+ 'type': 'executable',
+ 'sources': [
+ 'dirname_test.cc',
+ ],
+ 'dependencies': [
+ 'dirname',
+ '<(EXTERNALS):gtest',
+ ],
+ },
+ {
+ 'target_name': 'aos_strerror',
+ 'type': 'static_library',
+ 'sources': [
+ 'aos_strerror.cc',
+ ],
+ },
+ {
+ 'target_name': 'aos_strerror_test',
+ 'type': 'executable',
+ 'sources': [
+ 'aos_strerror_test.cc',
+ ],
+ 'dependencies': [
+ 'aos_strerror',
+ '<(EXTERNALS):gtest',
+ ],
+ },
+ ],
+}
diff --git a/aos/common/logging/logging.h b/aos/common/logging/logging.h
index 49828bf..e6e14f9 100644
--- a/aos/common/logging/logging.h
+++ b/aos/common/logging/logging.h
@@ -11,7 +11,7 @@
#include <errno.h>
#include "aos/common/macros.h"
-#include "aos/common/util/aos_strerror.h"
+#include "aos/common/libc/aos_strerror.h"
#ifdef __cplusplus
extern "C" {
diff --git a/aos/common/once_test.cc b/aos/common/once_test.cc
index 7c6e5de..3773b5b 100644
--- a/aos/common/once_test.cc
+++ b/aos/common/once_test.cc
@@ -1,7 +1,7 @@
#include "aos/common/once.h"
-#include "stdlib.h"
-#include "limits.h"
+#include <stdlib.h>
+#include <limits.h>
#include "gtest/gtest.h"
@@ -12,7 +12,7 @@
public:
static int *Function() {
++times_run_;
- value_ = rand() % INT_MAX;
+ value_ = 971 + times_run_;
return &value_;
}
@@ -94,7 +94,8 @@
int second_result = 0;
int *SecondFunction() {
- second_result = rand() % INT_MAX;
+ static int result = 254;
+ second_result = ++result;
return &second_result;
}
diff --git a/aos/common/time_test.cc b/aos/common/time_test.cc
index c073e9e..af6db05 100644
--- a/aos/common/time_test.cc
+++ b/aos/common/time_test.cc
@@ -34,19 +34,21 @@
// longer than it should the second time, where it actually matters)
SleepFor(Time(0, Time::kNSecInSec / 10));
Time start = Time::Now();
- SleepFor(Time(0, Time::kNSecInSec * 2 / 10));
- EXPECT_TRUE(MACRO_DARG((Time::Now() - start)
- .IsWithin(Time(0, Time::kNSecInSec * 2 / 10),
- Time::kNSecInSec / 1000)));
+ static constexpr Time kSleepTime = Time(0, Time::kNSecInSec * 2 / 10);
+ SleepFor(kSleepTime);
+ Time difference = Time::Now() - start;
+ EXPECT_GE(difference, kSleepTime);
+ EXPECT_LT(difference, kSleepTime + Time(0, Time::kNSecInSec / 100));
}
TEST(TimeTest, AbsoluteSleep) {
Time start = Time::Now();
SleepFor(Time(0, Time::kNSecInSec / 10));
- SleepUntil((start + Time(0, Time::kNSecInSec * 2 / 10)));
- EXPECT_TRUE(MACRO_DARG((Time::Now() - start)
- .IsWithin(Time(0, Time::kNSecInSec * 2 / 10),
- Time::kNSecInSec / 1000)));
+ static constexpr Time kSleepTime = Time(0, Time::kNSecInSec * 2 / 10);
+ SleepUntil(start + kSleepTime);
+ Time difference = Time::Now() - start;
+ EXPECT_GE(difference, kSleepTime);
+ EXPECT_LT(difference, kSleepTime + Time(0, Time::kNSecInSec / 100));
}
TEST(TimeTest, Addition) {
diff --git a/aos/common/util/run_command.cc b/aos/common/util/run_command.cc
new file mode 100644
index 0000000..a4a19d8
--- /dev/null
+++ b/aos/common/util/run_command.cc
@@ -0,0 +1,73 @@
+#include "aos/common/util/run_command.h"
+
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+
+#include "aos/common/logging/logging.h"
+
+namespace aos {
+namespace util {
+namespace {
+
+// RAII class to block SIGCHLD and then restore it on destruction.
+class BlockSIGCHLD {
+ public:
+ BlockSIGCHLD() {
+ sigset_t to_block;
+ sigemptyset(&to_block);
+ sigaddset(&to_block, SIGCHLD);
+ if (sigprocmask(SIG_BLOCK, &to_block, &original_blocked_) == -1) {
+ PLOG(FATAL, "sigprocmask(SIG_BLOCK, %p, %p) failed",
+ &to_block, &original_blocked_);
+ }
+ }
+ ~BlockSIGCHLD() {
+ if (sigprocmask(SIG_SETMASK, &original_blocked_, nullptr) == -1) {
+ PLOG(FATAL, "sigprocmask(SIG_SETMASK, %p, nullptr) failed",
+ &original_blocked_);
+ }
+ }
+
+ private:
+ sigset_t original_blocked_;
+};
+
+} // namespace
+
+int RunCommand(const char *command) {
+ BlockSIGCHLD blocker;
+ const pid_t pid = fork();
+ switch (pid) {
+ case 0: // in child
+ {
+ int new_stdin = open("/dev/null", O_RDONLY);
+ if (new_stdin == -1) _exit(127);
+ int new_stdout = open("/dev/null", O_WRONLY);
+ if (new_stdout == -1) _exit(127);
+ int new_stderr = open("/dev/null", O_WRONLY);
+ if (new_stderr == -1) _exit(127);
+ if (dup2(new_stdin, 0) != 0) _exit(127);
+ if (dup2(new_stdout, 1) != 1) _exit(127);
+ if (dup2(new_stderr, 2) != 2) _exit(127);
+ execl("/bin/sh", "sh", "-c", command, nullptr);
+ _exit(127);
+ }
+ case -1:
+ return -1;
+ default:
+ int stat;
+ while (waitpid(pid, &stat, 0) == -1) {
+ if (errno != EINTR) {
+ return -1;
+ }
+ }
+ return stat;
+ }
+}
+
+} // namespace util
+} // namespace aos
diff --git a/aos/common/util/run_command.h b/aos/common/util/run_command.h
new file mode 100644
index 0000000..d116481
--- /dev/null
+++ b/aos/common/util/run_command.h
@@ -0,0 +1,17 @@
+#ifndef AOS_COMMON_UTIL_RUN_COMMAND_H_
+#define AOS_COMMON_UTIL_RUN_COMMAND_H_
+
+namespace aos {
+namespace util {
+
+// Improved replacement for system(3). Doesn't block signals like system(3) and
+// is thread-safe. Also makes sure all 3 standard streams are /dev/null.
+//
+// This means that it passes command to `/bin/sh -c` and returns -1 or a status
+// like from wait(2).
+int RunCommand(const char *command);
+
+} // namespace util
+} // namespace aos
+
+#endif // AOS_COMMON_UTIL_RUN_COMMAND_H_
diff --git a/aos/common/util/run_command_test.cc b/aos/common/util/run_command_test.cc
new file mode 100644
index 0000000..b440e47
--- /dev/null
+++ b/aos/common/util/run_command_test.cc
@@ -0,0 +1,39 @@
+#include "aos/common/util/run_command.h"
+
+#include "gtest/gtest.h"
+
+namespace aos {
+namespace util {
+namespace testing {
+
+TEST(RunCommandTest, True) {
+ int result = RunCommand("true");
+ ASSERT_NE(-1, result);
+ ASSERT_TRUE(WIFEXITED(result));
+ EXPECT_EQ(0, WEXITSTATUS(result));
+}
+
+TEST(RunCommandTest, False) {
+ int result = RunCommand("false");
+ ASSERT_NE(-1, result);
+ ASSERT_TRUE(WIFEXITED(result));
+ EXPECT_EQ(1, WEXITSTATUS(result));
+}
+
+TEST(RunCommandTest, CommandNotFound) {
+ int result = RunCommand("ajflkjasdlfa");
+ ASSERT_NE(-1, result);
+ ASSERT_TRUE(WIFEXITED(result));
+ EXPECT_EQ(127, WEXITSTATUS(result));
+}
+
+TEST(RunCommandTest, KilledBySignal) {
+ int result = RunCommand("kill -QUIT $$");
+ ASSERT_NE(-1, result);
+ ASSERT_TRUE(WIFSIGNALED(result));
+ EXPECT_EQ(SIGQUIT, WTERMSIG(result));
+}
+
+} // namespace testing
+} // namespace util
+} // namespace aos
diff --git a/aos/common/util/util.gyp b/aos/common/util/util.gyp
index 9f3e1c6..7c0adf3 100644
--- a/aos/common/util/util.gyp
+++ b/aos/common/util/util.gyp
@@ -1,6 +1,28 @@
{
'targets': [
{
+ 'target_name': 'run_command',
+ 'type': 'static_library',
+ 'sources': [
+ 'run_command.cc',
+ ],
+ 'dependencies': [
+ '<(AOS)/build/aos.gyp:logging_interface',
+ ],
+ },
+ {
+ 'target_name': 'run_command_test',
+ 'type': 'executable',
+ 'sources': [
+ 'run_command_test.cc',
+ ],
+ 'dependencies': [
+ 'run_command',
+ '<(EXTERNALS):gtest',
+ '<(AOS)/build/aos.gyp:logging',
+ ],
+ },
+ {
'target_name': 'death_test_log_implementation',
'type': 'static_library',
'sources': [
@@ -14,13 +36,6 @@
],
},
{
- 'target_name': 'aos_strerror',
- 'type': 'static_library',
- 'sources': [
- 'aos_strerror.cc',
- ],
- },
- {
'target_name': 'inet_addr',
'type': 'static_library',
'sources': [
diff --git a/aos/linux_code/core.cc b/aos/linux_code/core.cc
index a9a58d1..5b580ef 100644
--- a/aos/linux_code/core.cc
+++ b/aos/linux_code/core.cc
@@ -1,3 +1,4 @@
+#include <sys/wait.h>
#include <sys/select.h>
#include <stdlib.h>
#include <stdio.h>
@@ -7,6 +8,7 @@
#include <string>
#include "aos/linux_code/init.h"
+#include "aos/common/util/run_command.h"
// Initializes shared memory. This is the only file that will create the shared
// memory file if it doesn't already exist (and set everything up).
@@ -17,8 +19,10 @@
aos::InitCreate();
if (argc > 1) {
- if (system((std::string("touch '") + argv[1] + "'").c_str()) != 0) {
- fprintf(stderr, "`touch '%s'` failed\n", argv[1]);
+ const int result = ::aos::util::RunCommand(
+ (std::string("touch '") + argv[1] + "'").c_str());
+ if (result == -1 || !WIFEXITED(result) || WEXITSTATUS(result) != 0) {
+ fprintf(stderr, "`touch '%s'` failed; result = %x\n", argv[1], result);
exit(EXIT_FAILURE);
}
}
diff --git a/aos/linux_code/ipc_lib/ipc_lib.gyp b/aos/linux_code/ipc_lib/ipc_lib.gyp
index 9b1e712..3c0c9fa 100644
--- a/aos/linux_code/ipc_lib/ipc_lib.gyp
+++ b/aos/linux_code/ipc_lib/ipc_lib.gyp
@@ -80,6 +80,8 @@
'<(AOS)/common/common.gyp:mutex',
'core_lib',
'<(AOS)/common/common.gyp:die',
+ '<(AOS)/common/libc/libc.gyp:dirname',
+ '<(AOS)/common/libc/libc.gyp:aos_strsignal',
],
'variables': {
'is_special_test': 1,
diff --git a/aos/linux_code/ipc_lib/ipc_stress_test.cc b/aos/linux_code/ipc_lib/ipc_stress_test.cc
index c1dcb83..58d88b3 100644
--- a/aos/linux_code/ipc_lib/ipc_stress_test.cc
+++ b/aos/linux_code/ipc_lib/ipc_stress_test.cc
@@ -15,6 +15,8 @@
#include "aos/common/mutex.h"
#include "aos/linux_code/ipc_lib/core_lib.h"
#include "aos/common/die.h"
+#include "aos/common/libc/dirname.h"
+#include "aos/common/libc/aos_strsignal.h"
// This runs all of the IPC-related tests in a bunch of parallel processes for a
// while and makes sure that they don't fail. It also captures the stdout and
@@ -175,7 +177,7 @@
} else if (WIFSIGNALED(status)) {
MutexLocker sync(&shared->output_mutex);
fprintf(stderr, "Test %s terminated by signal %d: %s.\n", (*test)[0],
- WTERMSIG(status), strsignal(WTERMSIG(status)));
+ WTERMSIG(status), aos_strsignal(WTERMSIG(status)));
fputs(output.c_str(), stderr);
} else {
assert(WIFSTOPPED(status));
@@ -210,12 +212,10 @@
Shared *shared = static_cast<Shared *>(shm_malloc(sizeof(Shared)));
new (shared) Shared(time::Time::Now() + kTestTime);
- char *temp = strdup(argv[0]);
if (asprintf(const_cast<char **>(&shared->path),
- "%s/../tests", dirname(temp)) == -1) {
+ "%s/../tests", ::aos::libc::Dirname(argv[0]).c_str()) == -1) {
PDie("asprintf failed");
}
- free(temp);
for (int i = 0; i < kTesters; ++i) {
Run(shared);
diff --git a/aos/linux_code/linux_code.gyp b/aos/linux_code/linux_code.gyp
index cbae81b..74b788d 100644
--- a/aos/linux_code/linux_code.gyp
+++ b/aos/linux_code/linux_code.gyp
@@ -31,6 +31,7 @@
],
'dependencies': [
'init',
+ '<(AOS)/common/util/util.gyp:run_command',
],
},
],
diff --git a/aos/linux_code/starter/starter.cc b/aos/linux_code/starter/starter.cc
index 800acd9..59aa88d 100644
--- a/aos/linux_code/starter/starter.cc
+++ b/aos/linux_code/starter/starter.cc
@@ -1,3 +1,8 @@
+// This has to come before anybody drags in <stdlib.h> or else we end up with
+// the wrong version of WIFEXITED etc (for one thing, they don't const-qualify
+// their casts) (sometimes at least).
+#include <sys/wait.h>
+
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
@@ -10,7 +15,6 @@
#include <stdint.h>
#include <errno.h>
#include <string.h>
-#include <sys/wait.h>
#include <inttypes.h>
#include <map>
@@ -32,6 +36,8 @@
#include "aos/common/unique_malloc_ptr.h"
#include "aos/common/time.h"
#include "aos/common/once.h"
+#include "aos/common/libc/aos_strsignal.h"
+#include "aos/common/util/run_command.h"
// This is the main piece of code that starts all of the rest of the code and
// restarts it when the binaries are modified.
@@ -669,8 +675,7 @@
// want it to stop, so it stopping isn't a WARNING.
LOG((status == SIGTERM) ? DEBUG : WARNING,
"child %d (%s) was killed by signal %d (%s)\n",
- pid, child->name(), status,
- strsignal(status));
+ pid, child->name(), status, aos_strsignal(status));
break;
case CLD_STOPPED:
LOG(WARNING, "child %d (%s) was stopped by signal %d "
@@ -731,8 +736,13 @@
std::string core_touch_file = "/tmp/starter.";
core_touch_file += std::to_string(static_cast<intmax_t>(getpid()));
core_touch_file += ".core_touch_file";
- if (system(("touch '" + core_touch_file + "'").c_str()) != 0) {
+ const int result =
+ ::aos::util::RunCommand(("touch '" + core_touch_file + "'").c_str());
+ if (result == -1) {
PLOG(FATAL, "running `touch '%s'` failed\n", core_touch_file.c_str());
+ } else if (!WIFEXITED(result) || WEXITSTATUS(result) != 0) {
+ LOG(FATAL, "`touch '%s'` gave result %x\n", core_touch_file.c_str(),
+ result);
}
FileWatch core_touch_file_watch(core_touch_file, Run, NULL);
core = unique_ptr<Child>(
diff --git a/aos/linux_code/starter/starter.gyp b/aos/linux_code/starter/starter.gyp
index 0464d58..5d86ea4 100644
--- a/aos/linux_code/starter/starter.gyp
+++ b/aos/linux_code/starter/starter.gyp
@@ -25,6 +25,8 @@
'<(AOS)/build/aos.gyp:logging',
'<(AOS)/common/common.gyp:once',
'<(AOS)/common/common.gyp:time',
+ '<(AOS)/common/libc/libc.gyp:aos_strsignal',
+ '<(AOS)/common/util/util.gyp:run_command',
],
'copies': [
{
diff --git a/bbb_cape/src/bbb/bbb.gyp b/bbb_cape/src/bbb/bbb.gyp
index e154cef..e5b19b0 100644
--- a/bbb_cape/src/bbb/bbb.gyp
+++ b/bbb_cape/src/bbb/bbb.gyp
@@ -54,6 +54,7 @@
'dependencies': [
'<(AOS)/build/aos.gyp:logging',
'<(AOS)/common/common.gyp:time',
+ '<(AOS)/common/util/util.gyp:run_command',
],
},
{
@@ -112,6 +113,7 @@
'crc',
'byte_io',
'<(AOS)/common/util/util.gyp:log_interval',
+ '<(AOS)/common/util/util.gyp:run_command',
],
'export_dependent_settings': [
'<(AOS)/build/aos.gyp:logging',
diff --git a/bbb_cape/src/bbb/export_uart.cc b/bbb_cape/src/bbb/export_uart.cc
index 3d2b24f..bcb1a7c 100644
--- a/bbb_cape/src/bbb/export_uart.cc
+++ b/bbb_cape/src/bbb/export_uart.cc
@@ -3,9 +3,11 @@
#include <unistd.h>
#include <errno.h>
#include <string.h>
+#include <sys/wait.h>
#include "aos/common/logging/logging.h"
#include "aos/common/time.h"
+#include "aos/common/util/run_command.h"
namespace bbb {
namespace {
@@ -27,11 +29,15 @@
void ExportUart() {
if (easy_access(device)) {
LOG(INFO, "unexporting BB-UART1\n");
- if (system("bash -c 'echo -$(cat /sys/devices/bone_capemgr.*/slots"
- " | fgrep BB-UART1"
- " | cut -d : -f 1 | tr -d \" \")"
- " > /sys/devices/bone_capemgr.*/slots'") == -1) {
- PLOG(FATAL, "system([disable OMAP UART]) failed");
+ const int result = ::aos::util::RunCommand(
+ "bash -c 'echo -$(cat /sys/devices/bone_capemgr.*/slots"
+ " | fgrep BB-UART1"
+ " | cut -d : -f 1 | tr -d \" \")"
+ " > /sys/devices/bone_capemgr.*/slots'");
+ if (result == -1) {
+ PLOG(FATAL, "RunCommand([disable OMAP UART]) failed");
+ } else if (!WIFEXITED(result) || WEXITSTATUS(result) != 0) {
+ LOG(FATAL, "command to disable OMAP UART failed; result = %x\n", result);
}
while (easy_access(device)) {
LOG(DEBUG, "waiting for BB-UART1 to be unexported\n");
@@ -42,10 +48,12 @@
LOG(INFO, "exporting BB-UART1\n");
// 2 strings to work around a VIM bug where the indenter locks up when they're
// combined as 1...
- if (system("bash -c 'echo BB-UART1 > /sys/devices/bone_capemgr.*"
- "/slots'") ==
- -1) {
- PLOG(FATAL, "system([enable OMAP UART]) failed");
+ const int result = ::aos::util::RunCommand(
+ "bash -c 'echo BB-UART1 > /sys/devices/bone_capemgr.*" "/slots'");
+ if (result == -1) {
+ PLOG(FATAL, "RunCommand([enable OMAP UART]) failed");
+ } else if (!WIFEXITED(result) || WEXITSTATUS(result) != 0) {
+ LOG(FATAL, "command to enable OMAP UART failed; result = %x\n", result);
}
while (!easy_access(device)) {
LOG(DEBUG, "waiting for BB-UART1 to be exported\n");
diff --git a/bbb_cape/src/bbb/packet_finder.cc b/bbb_cape/src/bbb/packet_finder.cc
index b9f1d2f..f3ca2b5 100644
--- a/bbb_cape/src/bbb/packet_finder.cc
+++ b/bbb_cape/src/bbb/packet_finder.cc
@@ -1,3 +1,8 @@
+// This has to come before anybody drags in <stdlib.h> or else we end up with
+// the wrong version of WIFEXITED etc (for one thing, they don't const-qualify
+// their casts) (sometimes at least).
+#include <sys/wait.h>
+
#include "bbb/packet_finder.h"
#include <inttypes.h>
@@ -8,6 +13,7 @@
#include <algorithm>
#include "aos/common/logging/logging.h"
+#include "aos/common/util/run_command.h"
#include "cape/cows.h"
#include "bbb/crc.h"
@@ -65,14 +71,13 @@
// Iff we're root.
if (getuid() == 0) {
// TODO(brians): Do this cleanly.
- int chrt_result =
- system("chrt -o 0 bash -c 'chrt -r -p 55"
- " $(pgrep irq/89)'");
+ const int chrt_result = ::aos::util::RunCommand(
+ "chrt -o 0 bash -c 'chrt -r -p 55 $(pgrep irq/89)'");
if (chrt_result == -1) {
- LOG(FATAL, "system(chrt -r -p 55 the_irq) failed\n");
+ LOG(FATAL, "RunCommand(chrt -r -p 55 the_irq) failed\n");
} else if (!WIFEXITED(chrt_result) || WEXITSTATUS(chrt_result) != 0) {
- LOG(FATAL, "$(chrt -r -p 55 the_irq) failed, return value = %d\n",
- WEXITSTATUS(chrt_result));
+ LOG(FATAL, "$(chrt -r -p 55 the_irq) failed; result = %x\n",
+ chrt_result);
}
} else {
LOG(WARNING, "not root, so not increasing priority of the IRQ\n");
diff --git a/bbb_cape/src/flasher/flasher.gyp b/bbb_cape/src/flasher/flasher.gyp
index fba1477..09e4ccf 100644
--- a/bbb_cape/src/flasher/flasher.gyp
+++ b/bbb_cape/src/flasher/flasher.gyp
@@ -19,6 +19,7 @@
'<(DEPTH)/bbb_cape/src/bbb/bbb.gyp:gpios',
'<(DEPTH)/bbb_cape/src/bbb/bbb.gyp:export_uart',
'<(AOS)/common/common.gyp:time',
+ '<(AOS)/common/libc/libc.gyp:dirname',
],
},
],
diff --git a/bbb_cape/src/flasher/stm32_flasher.cc b/bbb_cape/src/flasher/stm32_flasher.cc
index d9d87fe..639908a 100644
--- a/bbb_cape/src/flasher/stm32_flasher.cc
+++ b/bbb_cape/src/flasher/stm32_flasher.cc
@@ -9,6 +9,7 @@
#include "aos/common/logging/logging.h"
#include "aos/common/logging/logging_impl.h"
#include "aos/common/time.h"
+#include "aos/common/libc/dirname.h"
extern "C" {
#include "stm32flash/parsers/parser.h"
@@ -51,9 +52,9 @@
serial_baud_t baud_rate = SERIAL_BAUD_57600;
- ::std::string filename =
- ::std::string(dirname(strdup(argv[0]))) +
- "/../../../bbb_cape/src/cape/.obj/" + target + ".hex";
+ ::std::string filename = ::aos::libc::Dirname(argv[0]) +
+ "/../../../bbb_cape/src/cape/.obj/" + target +
+ ".hex";
int file = open(filename.c_str(), O_RDONLY);
if (file == -1) {