removed lots of asserts
Some of them relied on the side effects of evaluating their argument but
for most of them it just makes more sense to use CHECK.
diff --git a/aos/common/condition_test.cc b/aos/common/condition_test.cc
index 65a1028..6914a8b 100644
--- a/aos/common/condition_test.cc
+++ b/aos/common/condition_test.cc
@@ -75,7 +75,7 @@
new (shared_) Shared();
}
~ConditionTestProcess() {
- assert(child_ == -1);
+ CHECK_EQ(child_, -1);
}
void Start() {
@@ -87,7 +87,7 @@
Run();
exit(EXIT_SUCCESS);
} else { // in parent
- assert(child_ != -1);
+ CHECK_NE(child_, -1);
shared_->ready.Lock();
@@ -170,16 +170,16 @@
}
void Join() {
- assert(child_ != -1);
+ CHECK_NE(child_, -1);
int status;
do {
- assert(waitpid(child_, &status, 0) == child_);
+ CHECK_EQ(waitpid(child_, &status, 0), child_);
} while (!(WIFEXITED(status) || WIFSIGNALED(status)));
child_ = -1;
}
void Kill() {
- assert(child_ != -1);
- assert(kill(child_, SIGTERM) == 0);
+ CHECK_NE(child_, -1);
+ PCHECK(kill(child_, SIGTERM));
Join();
}
diff --git a/aos/common/input/driver_station_data.h b/aos/common/input/driver_station_data.h
index dca33f9..4bef0b6 100644
--- a/aos/common/input/driver_station_data.h
+++ b/aos/common/input/driver_station_data.h
@@ -4,8 +4,6 @@
// This file defines several types to support nicely looking at the data
// received from the driver's station.
-#include <assert.h>
-
#include <memory>
#include "aos/externals/WPILib/WPILib/NetworkRobot/NetworkRobotValues.h"
diff --git a/aos/common/libc/aos_strerror.cc b/aos/common/libc/aos_strerror.cc
index d5fc2b4..4bca500 100644
--- a/aos/common/libc/aos_strerror.cc
+++ b/aos/common/libc/aos_strerror.cc
@@ -26,7 +26,8 @@
__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);
+ const int r = snprintf(buffer, kBufferSize, "Unknown error %d", error);
+ assert(r > 0);
}
return buffer;
}
diff --git a/aos/common/logging/logging.h b/aos/common/logging/logging.h
index e6e14f9..c54e434 100644
--- a/aos/common/logging/logging.h
+++ b/aos/common/logging/logging.h
@@ -131,7 +131,6 @@
// don't support streaming in extra text. Some of the implementation is borrowed
// from there too.
// They all LOG(FATAL) with a helpful message when the check fails.
-// TODO(brians): Replace assert with CHECK
// Portions copyright (c) 1999, Google Inc.
// All rights reserved.
//
@@ -221,6 +220,18 @@
// initializer lists.
#define CHECK_NOTNULL(val) ::aos::CheckNotNull(STRINGIFY(val), val)
+inline int CheckSyscall(const char *syscall_string, int value) {
+ if (__builtin_expect(value == -1, false)) {
+ PLOG(FATAL, "%s failed", syscall_string);
+ }
+ return value;
+}
+
+// Check that syscall does not return -1. If it does, PLOG(FATAL)s. This is
+// useful for quickly checking syscalls where it's not very useful to print out
+// the values of any of the arguments.
+#define PCHECK(syscall) ::aos::CheckSyscall(STRINGIFY(syscall), syscall)
+
} // namespace aos
#endif // __cplusplus
diff --git a/aos/common/logging/logging_impl.cc b/aos/common/logging/logging_impl.cc
index 969c0ee..8aa66f5 100644
--- a/aos/common/logging/logging_impl.cc
+++ b/aos/common/logging/logging_impl.cc
@@ -1,6 +1,5 @@
#include "aos/common/logging/logging_impl.h"
-#include <assert.h>
#include <stdarg.h>
#include "aos/common/time.h"
diff --git a/aos/common/logging/logging_impl.h b/aos/common/logging/logging_impl.h
index f24ce6a..35e24a0 100644
--- a/aos/common/logging/logging_impl.h
+++ b/aos/common/logging/logging_impl.h
@@ -351,6 +351,7 @@
// Runs the given function with the current LogImplementation (handles switching
// it out while running function etc).
+// levels is how many LogImplementations to not use off the stack.
void RunWithCurrentImplementation(
int levels, ::std::function<void(LogImplementation *)> function);
diff --git a/aos/common/logging/logging_impl_test.cc b/aos/common/logging/logging_impl_test.cc
index 34bcdd1..8331c37 100644
--- a/aos/common/logging/logging_impl_test.cc
+++ b/aos/common/logging/logging_impl_test.cc
@@ -1,10 +1,11 @@
#include <string>
+#include <inttypes.h>
+
#include "gtest/gtest.h"
#include "aos/common/logging/logging_impl.h"
#include "aos/common/time.h"
-#include <inttypes.h>
using ::testing::AssertionResult;
using ::testing::AssertionSuccess;
@@ -19,6 +20,11 @@
virtual void DoLog(log_level level, const char *format, va_list ap) {
internal::FillInMessage(level, format, ap, &message_);
+ if (level == FATAL) {
+ internal::PrintMessage(stderr, message_);
+ abort();
+ }
+
used_ = true;
}
@@ -82,8 +88,6 @@
first = false;
Init();
- // We'll end up with several of them stacked up, but it really doesn't
- // matter.
AddImplementation(log_implementation = new TestLogImplementation());
}
@@ -133,13 +137,20 @@
EXPECT_TRUE(WasLogged(WARNING, expected.str()));
}
-#ifndef __VXWORKS__
TEST_F(LoggingDeathTest, Fatal) {
ASSERT_EXIT(LOG(FATAL, "this should crash it\n"),
::testing::KilledBySignal(SIGABRT),
"this should crash it");
}
-#endif
+
+TEST_F(LoggingDeathTest, PCHECK) {
+ EXPECT_DEATH(PCHECK(fprintf(stdin, "nope")),
+ ".*fprintf\\(stdin, \"nope\"\\).*failed.*");
+}
+
+TEST_F(LoggingTest, PCHECK) {
+ EXPECT_EQ(7, PCHECK(printf("abc123\n")));
+}
TEST_F(LoggingTest, PrintfDirectives) {
LOG(INFO, "test log %%1 %%d\n");
diff --git a/aos/common/logging/logging_interface.cc b/aos/common/logging/logging_interface.cc
index e4c6fcd..0bec52c 100644
--- a/aos/common/logging/logging_interface.cc
+++ b/aos/common/logging/logging_interface.cc
@@ -1,6 +1,5 @@
#include "aos/common/logging/logging_impl.h"
-#include <assert.h>
#include <stdarg.h>
#include <string.h>
@@ -47,7 +46,6 @@
LogImplementation *const top_implementation = context->implementation;
LogImplementation *new_implementation = top_implementation;
LogImplementation *implementation = NULL;
- assert(levels >= 1);
for (int i = 0; i < levels; ++i) {
implementation = new_implementation;
if (new_implementation == NULL) {
diff --git a/aos/common/queue_testutils.cc b/aos/common/queue_testutils.cc
index ed89d4f..824cb89 100644
--- a/aos/common/queue_testutils.cc
+++ b/aos/common/queue_testutils.cc
@@ -4,7 +4,6 @@
#include <sys/mman.h>
#include <unistd.h>
#include <stdlib.h>
-#include <assert.h>
#include "gtest/gtest.h"
@@ -126,7 +125,7 @@
// "shared" memory.
void *memory = mmap(NULL, kCoreSize, PROT_READ | PROT_WRITE,
MAP_SHARED | MAP_ANONYMOUS, -1, 0);
- assert(memory != MAP_FAILED);
+ CHECK_NE(memory, MAP_FAILED);
aos_core_use_address_as_shared_mem(memory, kCoreSize);
@@ -134,7 +133,7 @@
}
GlobalCoreInstance::~GlobalCoreInstance() {
- assert(munmap(global_core->mem_struct, kCoreSize) == 0);
+ PCHECK(munmap(global_core->mem_struct, kCoreSize));
global_core = NULL;
}
@@ -143,7 +142,7 @@
}
void PreventExit() {
- assert(atexit(TerminateExitHandler) == 0);
+ CHECK_EQ(atexit(TerminateExitHandler), 0);
}
} // namespace testing
diff --git a/aos/common/queue_types_test.cc b/aos/common/queue_types_test.cc
index 96b7dfb..514499b 100644
--- a/aos/common/queue_types_test.cc
+++ b/aos/common/queue_types_test.cc
@@ -9,6 +9,7 @@
#include "aos/common/test_queue.q.h"
#include "aos/common/byteorder.h"
#include "aos/queue_primitives.h"
+#include "aos/common/logging/logging.h"
using ::aos::common::testing::Structure;
using ::aos::common::testing::MessageWithStructure;
@@ -160,7 +161,7 @@
input_bytes = sizeof(kData) + kExtraInputBytes;
to_network(&kData, input);
output_bytes = kString.size() + 1;
- assert(output_bytes <= sizeof(output));
+ CHECK_LE(output_bytes, sizeof(output));
ASSERT_TRUE(PrintField(output, &output_bytes, input, &input_bytes,
Structure::GetType()->fields[2]->type));
EXPECT_EQ(kExtraInputBytes, input_bytes);
@@ -195,7 +196,7 @@
", struct_float:8.560000}";
TEST_F(PrintMessageTest, Basic) {
- assert(sizeof(input) >= kTestMessage1.Size());
+ CHECK_GE(sizeof(input), kTestMessage1.Size());
input_bytes = kTestMessage1.Serialize(input);
output_bytes = sizeof(output);
ASSERT_TRUE(PrintMessage(output, &output_bytes, input, &input_bytes,
@@ -205,7 +206,7 @@
}
TEST_F(PrintMessageTest, OutputTooSmall) {
- assert(sizeof(input) >= kTestMessage1.Size());
+ CHECK_GE(sizeof(input), kTestMessage1.Size());
input_bytes = kTestMessage1.Serialize(input);
output_bytes = kTestMessage1String.size();
EXPECT_FALSE(PrintMessage(output, &output_bytes, input, &input_bytes,
@@ -220,7 +221,7 @@
}
TEST_F(PrintMessageTest, Structure) {
- assert(sizeof(input) >= kTestStructure1.Size());
+ CHECK_GE(sizeof(input), kTestStructure1.Size());
input_bytes = kTestStructure1.Serialize(input);
output_bytes = sizeof(output);
ASSERT_TRUE(PrintMessage(output, &output_bytes, input, &input_bytes,
diff --git a/aos/common/util/trapezoid_profile.cc b/aos/common/util/trapezoid_profile.cc
index 4c5f07d..4aa5285 100644
--- a/aos/common/util/trapezoid_profile.cc
+++ b/aos/common/util/trapezoid_profile.cc
@@ -1,7 +1,5 @@
#include "aos/common/util/trapezoid_profile.h"
-#include <assert.h>
-
#include "aos/common/logging/logging.h"
using ::Eigen::Matrix;
@@ -117,7 +115,7 @@
acceleration_;
}
- assert(top_velocity > -maximum_velocity_);
+ CHECK_GT(top_velocity, -maximum_velocity_);
deceleration_time_ = (goal_velocity - top_velocity) /
deceleration_;
diff --git a/aos/linux_code/ipc_lib/ipc_lib.gyp b/aos/linux_code/ipc_lib/ipc_lib.gyp
index 35091e5..9f0de33 100644
--- a/aos/linux_code/ipc_lib/ipc_lib.gyp
+++ b/aos/linux_code/ipc_lib/ipc_lib.gyp
@@ -87,6 +87,7 @@
'<(AOS)/common/common.gyp:die',
'<(AOS)/common/libc/libc.gyp:dirname',
'<(AOS)/common/libc/libc.gyp:aos_strsignal',
+ '<(AOS)/build/aos.gyp:logging',
],
'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 58d88b3..880d226 100644
--- a/aos/linux_code/ipc_lib/ipc_stress_test.cc
+++ b/aos/linux_code/ipc_lib/ipc_stress_test.cc
@@ -5,7 +5,6 @@
#include <sys/types.h>
#include <sys/wait.h>
#include <libgen.h>
-#include <assert.h>
#include <string>
@@ -17,6 +16,7 @@
#include "aos/common/die.h"
#include "aos/common/libc/dirname.h"
#include "aos/common/libc/aos_strsignal.h"
+#include "aos/common/logging/logging.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
@@ -180,7 +180,7 @@
WTERMSIG(status), aos_strsignal(WTERMSIG(status)));
fputs(output.c_str(), stderr);
} else {
- assert(WIFSTOPPED(status));
+ CHECK(WIFSTOPPED(status));
Die("Test %s was stopped.\n", (*test)[0]);
}
@@ -205,7 +205,10 @@
}
int Main(int argc, char **argv) {
- assert(argc >= 1);
+ if (argc < 1) {
+ fputs("need an argument\n", stderr);
+ return EXIT_FAILURE;
+ }
::aos::common::testing::GlobalCoreInstance global_core;
diff --git a/aos/linux_code/ipc_lib/queue.h b/aos/linux_code/ipc_lib/queue.h
index 31e8075..d27c6bf 100644
--- a/aos/linux_code/ipc_lib/queue.h
+++ b/aos/linux_code/ipc_lib/queue.h
@@ -1,8 +1,6 @@
#ifndef AOS_LINUX_CODE_IPC_LIB_QUEUE_H_
#define AOS_LINUX_CODE_IPC_LIB_QUEUE_H_
-#include <assert.h>
-
#include "aos/linux_code/ipc_lib/shared_mem.h"
#include "aos/common/mutex.h"
#include "aos/common/condition.h"
diff --git a/aos/linux_code/ipc_lib/raw_queue_test.cc b/aos/linux_code/ipc_lib/raw_queue_test.cc
index c85d36a..2267c92 100644
--- a/aos/linux_code/ipc_lib/raw_queue_test.cc
+++ b/aos/linux_code/ipc_lib/raw_queue_test.cc
@@ -161,7 +161,7 @@
std::unique_ptr<ForkedProcess> ForkExecute(void (*function)(T*), T *arg) {
mutex *lock = static_cast<mutex *>(shm_malloc_aligned(
sizeof(*lock), sizeof(int)));
- assert(mutex_lock(lock) == 0);
+ CHECK_EQ(mutex_lock(lock), 0);
const pid_t pid = fork();
switch (pid) {
case 0: // child
diff --git a/aos/linux_code/starter/netconsole.cc b/aos/linux_code/starter/netconsole.cc
index 4d32afe..c4ca5b9 100644
--- a/aos/linux_code/starter/netconsole.cc
+++ b/aos/linux_code/starter/netconsole.cc
@@ -7,7 +7,6 @@
#include <fcntl.h>
#include <pthread.h>
#include <sys/stat.h>
-#include <assert.h>
#include "aos/common/logging/logging_impl.h"
#include "aos/common/util/inet_addr.h"
@@ -33,8 +32,8 @@
char buffer[32768];
ssize_t position = 0;
while (true) {
- assert(position >= 0);
- assert(position <= static_cast<ssize_t>(sizeof(buffer)));
+ CHECK_GE(position, 0);
+ CHECK_LE(position, static_cast<ssize_t>(sizeof(buffer)));
if (position != sizeof(buffer)) {
ssize_t read_bytes;
bool good_data = true;
@@ -73,7 +72,7 @@
}
}
if (to_copy->source_address != nullptr) {
- assert(header.msg_namelen >= sizeof(struct sockaddr_in));
+ CHECK_GE(header.msg_namelen, sizeof(struct sockaddr_in));
if (to_copy->source_address->sin_port != hton<uint16_t>(0)) {
if (sender_address.sin_port !=
to_copy->source_address->sin_port) {
@@ -104,8 +103,8 @@
}
}
- assert(position >= 0);
- assert(position <= static_cast<ssize_t>(sizeof(buffer)));
+ CHECK_GE(position, 0);
+ CHECK_LE(position, static_cast<ssize_t>(sizeof(buffer)));
if (position > 0) {
ssize_t sent_bytes = write(to_copy->output, buffer, position);
if (sent_bytes == -1) {
diff --git a/aos/linux_code/starter/starter.cc b/aos/linux_code/starter/starter.cc
index 59aa88d..0fbb7ed 100644
--- a/aos/linux_code/starter/starter.cc
+++ b/aos/linux_code/starter/starter.cc
@@ -10,7 +10,6 @@
#include <sys/inotify.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
-#include <assert.h>
#include <signal.h>
#include <stdint.h>
#include <errno.h>
@@ -114,8 +113,8 @@
// After calling this method, this object won't really be doing much of
// anything besides possibly running its callback or something.
void RemoveWatch() {
- assert(watch_ != -1);
- assert(watch_to_remove_ == -1);
+ CHECK_NE(watch_, -1);
+ CHECK_EQ(watch_to_remove_, -1);
if (inotify_rm_watch(notify_fd, watch_) == -1) {
PLOG(WARNING, "inotify_rm_watch(%d, %d) failed", notify_fd, watch_);
@@ -139,7 +138,7 @@
void RemoveWatchFromMap() {
int watch = watch_to_remove_;
if (watch == -1) {
- assert(watch_ != -1);
+ CHECK_NE(watch, -1);
watch = watch_;
}
if (watchers[watch] != this) {
@@ -157,7 +156,7 @@
}
void CreateWatch() {
- assert(watch_ == -1);
+ CHECK_EQ(watch_, -1);
watch_ = inotify_add_watch(notify_fd, filename_.c_str(),
create_ ? IN_CREATE : (IN_ATTRIB |
IN_MODIFY |
@@ -227,7 +226,7 @@
// INotifyReadable calls this method whenever the watch for our file triggers.
void FileNotified(const char *filename) {
- assert(watch_ != -1);
+ CHECK_NE(watch_, -1);
LOG(DEBUG, "got a notification for %s\n", filename_.c_str());
if (!check_filename_.empty()) {