got rid of all uses of strerror
This required some minor refactoring of other things and there were some
other small cleanups I noticed along the way.
diff --git a/aos/common/common.gyp b/aos/common/common.gyp
index c989bf2..2581eaa 100644
--- a/aos/common/common.gyp
+++ b/aos/common/common.gyp
@@ -225,6 +225,12 @@
'sources': [
'die.cc',
],
+ 'dependencies': [
+ '<(AOS)/common/util/util.gyp:aos_strerror',
+ ],
+ 'export_dependent_settings': [
+ '<(AOS)/common/util/util.gyp:aos_strerror',
+ ],
},
{
'target_name': 'condition',
@@ -277,6 +283,7 @@
'mutex',
'die',
'<(AOS)/build/aos.gyp:logging',
+ '<(AOS)/common/util/util.gyp:death_test_log_implementation',
],
},
{
diff --git a/aos/common/die.cc b/aos/common/die.cc
index 4c579cd..e1b3b07 100644
--- a/aos/common/die.cc
+++ b/aos/common/die.cc
@@ -38,8 +38,8 @@
return r;
} else {
fprintf(stderr, "aos fatal: asprintf(%p, \"thingie with %%jd\", %jd)"
- " failed with %d (%s)\n", &filename,
- static_cast<intmax_t>(getpid()), errno, strerror(errno));
+ " failed with %d\n", &filename,
+ static_cast<intmax_t>(getpid()), errno);
return std::string();
}
#endif
@@ -68,8 +68,8 @@
vfprintf(error_file, format, args2);
fclose(error_file);
} else {
- fprintf(stderr, "aos fatal: fopen('%s', \"w\") failed with %d (%s)\n",
- filename.c_str(), errno, strerror(errno));
+ fprintf(stderr, "aos fatal: fopen('%s', \"w\") failed with %d\n",
+ filename.c_str(), errno);
}
}
}
diff --git a/aos/common/die.h b/aos/common/die.h
index 3c5cca9..05500be 100644
--- a/aos/common/die.h
+++ b/aos/common/die.h
@@ -4,6 +4,7 @@
#include <stdarg.h>
#include "aos/common/macros.h"
+#include "aos/common/util/aos_strerror.h"
namespace aos {
@@ -17,6 +18,16 @@
__attribute__((noreturn))
__attribute__((format(GOOD_PRINTF_FORMAT_TYPE, 1, 0)));
+
+// The same as Die except appends " because of %d (%s)" (formatted with errno
+// and aos_strerror(errno)) to the message.
+#define PDie(format, args...) \
+ do { \
+ const int error = errno; \
+ ::aos::Die(format " because of %d (%s)", ##args, error, \
+ aos_strerror(error)); \
+ } while (false);
+
// Turns on (or off) "test mode", where (V)Die doesn't write out files and
// doesn't print to stdout.
// Test mode defaults to false.
diff --git a/aos/common/logging/logging.h b/aos/common/logging/logging.h
index 03f3e31..49828bf 100644
--- a/aos/common/logging/logging.h
+++ b/aos/common/logging/logging.h
@@ -7,8 +7,11 @@
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
#include "aos/common/macros.h"
+#include "aos/common/util/aos_strerror.h"
#ifdef __cplusplus
extern "C" {
@@ -36,6 +39,7 @@
#ifdef __cplusplus
extern "C" {
#endif
+
// Actually implements the basic logging call.
// Does not check that level is valid.
void log_do(log_level level, const char *format, ...)
@@ -47,6 +51,7 @@
void log_uncork(int line, const char *function, log_level level,
const char *file, const char *format, ...)
__attribute__((format(GOOD_PRINTF_FORMAT_TYPE, 5, 6)));
+
#ifdef __cplusplus
}
#endif
@@ -74,6 +79,17 @@
} \
} while (0)
+// Same as LOG except appends " due to %d(%s)\n" (formatted with errno and
+// aos_strerror(errno)) to the message.
+#define PLOG(level, format, args...) PELOG(level, errno, format, ##args)
+
+// Like PLOG except allows specifying an error other than errno.
+#define PELOG(level, error_in, format, args...) \
+ do { \
+ const int error = error_in; \
+ LOG(level, format " due to %d(%s)\n", ##args, error, aos_strerror(error)); \
+ } while (0);
+
// Allows format to not be a string constant.
#define LOG_DYNAMIC(level, format, args...) \
do { \
diff --git a/aos/common/logging/logging_interface.cc b/aos/common/logging/logging_interface.cc
index ae8cc01..8c45afd 100644
--- a/aos/common/logging/logging_interface.cc
+++ b/aos/common/logging/logging_interface.cc
@@ -30,8 +30,8 @@
const int ret = vsnprintf(output, size, format, ap);
typedef ::std::common_type<typeof(ret), typeof(size)>::type RetType;
if (ret < 0) {
- LOG(FATAL, "vsnprintf(%p, %zd, %s, args) failed with %d (%s)\n",
- output, size, format, errno, strerror(errno));
+ PLOG(FATAL, "vsnprintf(%p, %zd, %s, args) failed",
+ output, size, format);
} else if (static_cast<RetType>(ret) >= static_cast<RetType>(size)) {
// Overwrite the '\0' at the end of the existing data and
// copy in the one on the end of continued.
diff --git a/aos/common/mutex_test.cpp b/aos/common/mutex_test.cpp
index f946dfa..2ab3e5c 100644
--- a/aos/common/mutex_test.cpp
+++ b/aos/common/mutex_test.cpp
@@ -11,6 +11,7 @@
#include "aos/linux_code/ipc_lib/aos_sync.h"
#include "aos/common/die.h"
+#include "aos/common/util/death_test_log_implementation.h"
namespace aos {
namespace testing {
@@ -49,12 +50,22 @@
TEST_F(MutexDeathTest, RepeatUnlock) {
test_mutex.Lock();
test_mutex.Unlock();
- EXPECT_DEATH(test_mutex.Unlock(), ".*multiple unlock.*");
+ EXPECT_DEATH(
+ {
+ logging::AddImplementation(new util::DeathTestLogImplementation());
+ test_mutex.Unlock();
+ },
+ ".*multiple unlock.*");
}
// Sees what happens if you unlock without ever locking (or unlocking) it.
TEST_F(MutexDeathTest, NeverLock) {
- EXPECT_DEATH(test_mutex.Unlock(), ".*multiple unlock.*");
+ EXPECT_DEATH(
+ {
+ logging::AddImplementation(new util::DeathTestLogImplementation());
+ test_mutex.Unlock();
+ },
+ ".*multiple unlock.*");
}
#endif
diff --git a/aos/common/network/receive_socket.cc b/aos/common/network/receive_socket.cc
index 76a7a80..c3a7710 100644
--- a/aos/common/network/receive_socket.cc
+++ b/aos/common/network/receive_socket.cc
@@ -23,8 +23,7 @@
if (bind(socket_, &addr_.addr,
sizeof(addr_)) == -1) {
- LOG(ERROR, "failed to bind to address '%s' because of %d: %s\n", localhost,
- errno, strerror(errno));
+ PLOG(ERROR, "failed to bind to address '%s'", localhost);
return last_ret_ = -1;
}
return last_ret_ = 0;
diff --git a/aos/common/network/send_socket.cc b/aos/common/network/send_socket.cc
index 03bc57c..9886893 100644
--- a/aos/common/network/send_socket.cc
+++ b/aos/common/network/send_socket.cc
@@ -20,8 +20,7 @@
}
if (connect(socket_, &addr_.addr, sizeof(addr_)) < 0) {
- LOG(ERROR, "couldn't connect to ip '%s' because of %d: %s\n", robot_ip,
- errno, strerror(errno));
+ PLOG(ERROR, "couldn't connect to ip '%s'", robot_ip);
return last_ret_ = 1;
}
diff --git a/aos/common/network/socket.cc b/aos/common/network/socket.cc
index 4f6687e..b674ff1 100644
--- a/aos/common/network/socket.cc
+++ b/aos/common/network/socket.cc
@@ -13,8 +13,7 @@
int Socket::Connect(NetworkPort port, const char *address, int type) {
last_ret_ = 0;
if ((socket_ = socket(AF_INET, type, 0)) < 0) {
- LOG(ERROR, "failed to create socket because of %d: %s\n",
- errno, strerror(errno));
+ PLOG(ERROR, "failed to create socket");
return last_ret_ = 1;
}
@@ -27,8 +26,7 @@
const int failure_return = -1;
#endif
if (inet_aton(address, &addr_.in.sin_addr) == failure_return) {
- LOG(ERROR, "Invalid IP address '%s' because of %d: %s\n", address,
- errno, strerror(errno));
+ PLOG(ERROR, "invalid IP address '%s'", address);
return last_ret_ = -1;
}
@@ -69,8 +67,8 @@
if (errno == EINTR) {
return last_ret_ = 0;
}
- LOG(FATAL, "select(FD_SETSIZE, %p, NULL, NULL, %p) failed with %d: %s\n",
- &fds, &timeout_timeval, errno, strerror(errno));
+ PLOG(FATAL, "select(FD_SETSIZE, %p, NULL, NULL, %p) failed",
+ &fds, &timeout_timeval);
}
}
diff --git a/aos/common/queue_testutils.cc b/aos/common/queue_testutils.cc
index 1b9b731..ed89d4f 100644
--- a/aos/common/queue_testutils.cc
+++ b/aos/common/queue_testutils.cc
@@ -128,7 +128,7 @@
MAP_SHARED | MAP_ANONYMOUS, -1, 0);
assert(memory != MAP_FAILED);
- assert(aos_core_use_address_as_shared_mem(memory, kCoreSize) == 0);
+ aos_core_use_address_as_shared_mem(memory, kCoreSize);
EnableTestLogging();
}
diff --git a/aos/common/scoped_fd.h b/aos/common/scoped_fd.h
index eb22f80..57ecdd8 100644
--- a/aos/common/scoped_fd.h
+++ b/aos/common/scoped_fd.h
@@ -30,8 +30,7 @@
void Close() {
if (fd_ != -1) {
if (close(fd_) == -1) {
- LOG(WARNING, "close(%d) failed with %d: %s\n", fd_,
- errno, strerror(errno));
+ PLOG(WARNING, "close(%d) failed", fd_);
}
}
}
diff --git a/aos/common/time.cc b/aos/common/time.cc
index 88802e3..aa75eae 100644
--- a/aos/common/time.cc
+++ b/aos/common/time.cc
@@ -1,9 +1,5 @@
#include "aos/common/time.h"
-#ifdef __VXWORKS__
-#include <taskLib.h>
-#endif
-#include <errno.h>
#include <string.h>
#include "aos/common/logging/logging.h"
@@ -34,8 +30,8 @@
Time NowImpl(clockid_t clock) {
timespec temp;
if (clock_gettime(clock, &temp) != 0) {
- LOG(FATAL, "clock_gettime(%jd, %p) failed with %d: %s\n",
- static_cast<uintmax_t>(clock), &temp, errno, strerror(errno));
+ PLOG(FATAL, "clock_gettime(%jd, %p) failed",
+ static_cast<uintmax_t>(clock), &temp);
}
return Time(temp);
}
@@ -192,53 +188,30 @@
}
void SleepFor(const Time &time, clockid_t clock) {
-#ifdef __VXWORKS__
- SleepUntil(Time::Now(clock) + time, clock);
-#else
timespec converted(time.ToTimespec()), remaining;
int failure = EINTR;
do {
// This checks whether the last time through the loop actually failed or got
// interrupted.
if (failure != EINTR) {
- LOG(FATAL, "clock_nanosleep(%jd, 0, %p, %p) returned %d: %s\n",
- static_cast<intmax_t>(clock), &converted, &remaining,
- failure, strerror(failure));
+ PELOG(FATAL, failure, "clock_nanosleep(%jd, 0, %p, %p) failed",
+ static_cast<intmax_t>(clock), &converted, &remaining);
}
failure = clock_nanosleep(clock, 0, &converted, &remaining);
memcpy(&converted, &remaining, sizeof(converted));
} while (failure != 0);
-#endif
}
void SleepUntil(const Time &time, clockid_t clock) {
-#ifdef __VXWORKS__
- if (clock != CLOCK_REALTIME) {
- LOG(FATAL, "vxworks only supports CLOCK_REALTIME\n");
- }
- // Vxworks nanosleep is definitely broken (fails horribly at doing remaining
- // right), and I don't really want to know how else it's broken, so I'm using
- // taskDelay instead because that's simpler.
- // The +1 is because sleep functions are supposed to sleep for at least the
- // requested amount, so we have to round up to the next clock tick.
- while (taskDelay((time - Time::Now(clock)).ToTicks() + 1) != 0) {
- if (errno != EINTR) {
- LOG(FATAL, "taskDelay(some ticks) failed with %d: %s\n",
- errno, strerror(errno));
- }
- }
-#else
timespec converted(time.ToTimespec());
int failure;
while ((failure = clock_nanosleep(clock, TIMER_ABSTIME,
&converted, NULL)) != 0) {
if (failure != EINTR) {
- LOG(FATAL, "clock_nanosleep(%jd, TIMER_ABSTIME, %p, NULL)"
- " returned %d: %s\n", static_cast<intmax_t>(clock), &converted,
- failure, strerror(failure));
+ PELOG(FATAL, failure, "clock_nanosleep(%jd, TIMER_ABSTIME, %p, NULL)"
+ " failed", static_cast<intmax_t>(clock), &converted);
}
}
-#endif
}
} // namespace time
diff --git a/aos/common/util/aos_strerror.cc b/aos/common/util/aos_strerror.cc
new file mode 100644
index 0000000..d2b2ca6
--- /dev/null
+++ b/aos/common/util/aos_strerror.cc
@@ -0,0 +1,42 @@
+#include "aos/common/util/aos_strerror.h"
+
+#include <assert.h>
+#include <sys/types.h>
+#include <string.h>
+#include <stdio.h>
+
+#include "aos/linux_code/thread_local.h"
+
+// This code uses an overloaded function to handle the result from either
+// version of strerror_r correctly without needing a way to get the choice out
+// of the compiler/glibc/whatever explicitly.
+
+namespace {
+
+const size_t kBufferSize = 128;
+
+// Handle the result from the GNU version of strerror_r. It never fails, so
+// that's pretty easy...
+__attribute__((unused))
+char *aos_strerror_handle_result(int /*error*/, char *ret, char * /*buffer*/) {
+ return ret;
+}
+
+// Handle the result from the POSIX version of strerror_r.
+__attribute__((unused))
+char *aos_strerror_handle_result(int error, int ret, char *buffer) {
+ if (ret != 0) {
+ assert(snprintf(buffer, kBufferSize, "Unknown error %d", error) > 0);
+ }
+ return buffer;
+}
+
+} // namespace
+
+char *aos_strerror(int error) {
+ static AOS_THREAD_LOCAL char buffer[kBufferSize];
+
+ // Call the overload for whichever version we're using.
+ return aos_strerror_handle_result(
+ error, strerror_r(error, buffer, sizeof(buffer)), buffer);
+}
diff --git a/aos/common/util/aos_strerror.h b/aos/common/util/aos_strerror.h
new file mode 100644
index 0000000..2fd6818
--- /dev/null
+++ b/aos/common/util/aos_strerror.h
@@ -0,0 +1,21 @@
+#ifndef AOS_COMMON_UTIL_AOS_STRERROR_H_
+#define AOS_COMMON_UTIL_AOS_STRERROR_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// Thread-safe version of strerror(3) (except it may change errno).
+//
+// 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);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif // AOS_COMMON_UTIL_AOS_STRERROR_H_
diff --git a/aos/common/util/death_test_log_implementation.h b/aos/common/util/death_test_log_implementation.h
new file mode 100644
index 0000000..c1b12c0
--- /dev/null
+++ b/aos/common/util/death_test_log_implementation.h
@@ -0,0 +1,27 @@
+#ifndef AOS_COMMON_UTIL_DEATH_TEST_LOG_IMPLEMENTATION_H_
+#define AOS_COMMON_UTIL_DEATH_TEST_LOG_IMPLEMENTATION_H_
+
+#include <stdlib.h>
+
+#include "aos/common/logging/logging_impl.h"
+
+namespace aos {
+namespace util {
+
+// Prints all FATAL messages to stderr and then abort(3)s before the regular
+// stuff can print out anything else. Ignores all other messages.
+// This is useful in death tests that expect a LOG(FATAL) to cause the death.
+class DeathTestLogImplementation : public logging::HandleMessageLogImplementation {
+ public:
+ virtual void HandleMessage(const logging::LogMessage &message) override {
+ if (message.level == FATAL) {
+ logging::internal::PrintMessage(stderr, message);
+ abort();
+ }
+ }
+};
+
+} // namespace util
+} // namespace aos
+
+#endif // AOS_COMMON_UTIL_DEATH_TEST_LOG_IMPLEMENTATION_H_
diff --git a/aos/common/util/util.gyp b/aos/common/util/util.gyp
index b20a7d9..9f3e1c6 100644
--- a/aos/common/util/util.gyp
+++ b/aos/common/util/util.gyp
@@ -1,6 +1,26 @@
{
'targets': [
{
+ 'target_name': 'death_test_log_implementation',
+ 'type': 'static_library',
+ 'sources': [
+ #'death_test_log_implementation',
+ ],
+ 'dependencies': [
+ '<(AOS)/build/aos.gyp:logging',
+ ],
+ 'export_dependent_settings': [
+ '<(AOS)/build/aos.gyp:logging',
+ ],
+ },
+ {
+ 'target_name': 'aos_strerror',
+ 'type': 'static_library',
+ 'sources': [
+ 'aos_strerror.cc',
+ ],
+ },
+ {
'target_name': 'inet_addr',
'type': 'static_library',
'sources': [