Address reviewer's comment.

* Make StackTrace size unsigned. * Don't output build_config.h into binary directory.
dmlc · Oct 26, 2018 · 80527a0 · 80527a0
1 parent 4015880
commit 80527a0
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 10 deletions.
diff --git a/include/dmlc/logging.h b/include/dmlc/logging.h
@@ -320,11 +320,11 @@ inline std::string Demangle(char const *msg_str) {
 }
 
 inline std::string StackTrace(
-    const int stack_size = DMLC_LOG_STACK_TRACE_SIZE) {
+    const size_t stack_size = DMLC_LOG_STACK_TRACE_SIZE) {
   using std::string;
   std::ostringstream stacktrace_os;
   std::vector<void*> stack(stack_size);
-  int nframes = backtrace(stack.data(), stack_size);
+  int nframes = backtrace(stack.data(), static_cast<int>(stack_size));
   stacktrace_os << "Stack trace returned " << nframes << " entries:" << std::endl;
   char **msgs = backtrace_symbols(stack.data(), nframes);
   if (msgs != nullptr) {
@@ -344,7 +344,7 @@ inline std::string demangle(char const* msg_str) {
   return std::string();
 }
 
-inline std::string StackTrace(const int stack_size = 0) {
+inline std::string StackTrace(const size_t stack_size = 0) {
   return std::string("stack traces not available when "
   "DMLC_LOG_STACK_TRACE is disabled at compile time.");
 }

diff --git a/test/unittest/CMakeLists.txt b/test/unittest/CMakeLists.txt
@@ -17,9 +17,8 @@ add_executable(${PROJECT_NAME}_unit_tests ${UNIT_TEST_SOURCE})
 set_property(TARGET ${PROJECT_NAME}_unit_tests
   PROPERTY RUNTIME_OUTPUT_DIRECTORY ${PRIVATE_RUNTIME_DIR})
 
-message(STATUS "${CMAKE_CURRENT_SOURCE_DIR}/build_config.h.in -> ${CMAKE_CURRENT_BINARY_DIR}/build_config.h")
-configure_file("${CMAKE_CURRENT_SOURCE_DIR}/build_config.h.in" "${CMAKE_CURRENT_BINARY_DIR}/build_config.h")
-target_include_directories(${PROJECT_NAME}_unit_tests PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
+message(STATUS "${CMAKE_CURRENT_SOURCE_DIR}/build_config.h.in -> ${CMAKE_CURRENT_SOURCE_DIR}/build_config.h")
+configure_file("${CMAKE_CURRENT_SOURCE_DIR}/build_config.h.in" "${CMAKE_CURRENT_SOURCE_DIR}/build_config.h")
 
 target_compile_definitions(${PROJECT_NAME}_unit_tests PRIVATE -DDMLC_UNIT_TESTS_USE_CMAKE)
 target_include_directories(${PROJECT_NAME}_unit_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
@@ -57,4 +56,4 @@ else()
     Threads::Threads)
 endif()
 
-add_test(AllTestsIn${PROJECT_NAME}UnitTests ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${PROJECT_NAME}_unit_tests)
+add_test(AllTestsIn${PROJECT_NAME}UnitTests ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${PROJECT_NAME}_unit_tests)
diff --git a/test/unittest/unittest_inputsplit.cc b/test/unittest/unittest_inputsplit.cc
@@ -147,7 +147,7 @@ TEST(InputSplit, test_split_libsvm_distributed) {
 #ifdef DMLC_UNIT_TESTS_USE_CMAKE
 /* Don't run the following when CMake is not used */
 
-#include <build_config.h>
+#include "./build_config.h"
 
 TEST(InputSplit, test_recordio) {
   dmlc::TemporaryDirectory tempdir;

diff --git a/test/unittest/unittest_logging.cc b/test/unittest/unittest_logging.cc
@@ -4,8 +4,6 @@
 #include <dmlc/logging.h>
 #include <gtest/gtest.h>
 
-#include <string>
-
 TEST(Logging, basics) {
   LOG(INFO) << "hello";
   LOG(ERROR) << "error";