From 6faf3588c1ef39f996df4679afab71ca7ec4dd00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steven!=20Ragnar=C3=B6k?= Date: Fri, 4 Oct 2019 11:47:20 -0700 Subject: [PATCH] Enable linters for noop and log4cxx. (#12) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While we are in here, fix up a few typos and cleanup the code to conform to the linting rules. Signed-off-by: Steven! Ragnarök Signed-off-by: Chris Lalancette --- rcl_logging_log4cxx/CMakeLists.txt | 9 +- rcl_logging_log4cxx/cmake/FindLog4cxx.cmake | 4 +- .../rcl_logging_log4cxx/logging_interface.h | 4 +- .../rcl_logging_log4cxx.cpp | 90 +++++++++---------- rcl_logging_noop/CMakeLists.txt | 4 + .../src/rcl_logging_noop/rcl_logging_noop.cpp | 14 +-- 6 files changed, 68 insertions(+), 57 deletions(-) diff --git a/rcl_logging_log4cxx/CMakeLists.txt b/rcl_logging_log4cxx/CMakeLists.txt index d805e31..40d6eaa 100644 --- a/rcl_logging_log4cxx/CMakeLists.txt +++ b/rcl_logging_log4cxx/CMakeLists.txt @@ -22,7 +22,7 @@ include_directories(${ament_INCLUDE_DIRS} include) if(NOT WIN32) add_compile_options(-Wall -Wextra -Wpedantic) else() - # Disable ddl-interface warnings for STL from log4cxx code, because we are already + # Disable dll-interface warnings for STL from log4cxx code, because we are already # forced to use the same compiler in Windows, as we are getting the binaries # for log4cxx from Chocolatey # https://web.archive.org/web/20130317015847/http://connect.microsoft.com/VisualStudio/feedback/details/696593/vc-10-vs-2010-basic-string-exports @@ -46,6 +46,13 @@ ament_target_dependencies(${PROJECT_NAME} target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_LOGGING_BUILDING_DLL") +if(BUILD_TESTING) + find_package(ament_lint_auto REQUIRED) + set(ament_cmake_lint_cmake_FOUND TRUE) + set(ament_cmake_copyright_FOUND TRUE) + ament_lint_auto_find_test_dependencies() +endif() + install(TARGETS ${PROJECT_NAME} ARCHIVE DESTINATION lib LIBRARY DESTINATION lib diff --git a/rcl_logging_log4cxx/cmake/FindLog4cxx.cmake b/rcl_logging_log4cxx/cmake/FindLog4cxx.cmake index e029f36..d5923ec 100644 --- a/rcl_logging_log4cxx/cmake/FindLog4cxx.cmake +++ b/rcl_logging_log4cxx/cmake/FindLog4cxx.cmake @@ -16,7 +16,7 @@ if( Log4cxx_DIR ) find_package( Log4cxx NO_MODULE ) elseif( NOT Log4cxx_FOUND ) message(STATUS "Searching for Log4cxx/logger.h") - if ( NOT WIN32 ) + if( NOT WIN32 ) find_path( Log4cxx_INCLUDE_DIR log4cxx/logger.h ) else() find_path( Log4cxx_INCLUDE_DIR log4cxx/logger.h HINTS "C:\\ProgramData\\chocolatey\\lib\\log4cxx\\include" ) @@ -24,7 +24,7 @@ elseif( NOT Log4cxx_FOUND ) set(Log4cxx_INCLUDE_DIRS ${Log4cxx_INCLUDE_DIR}) message(STATUS "Searching for libLog4cxx") - if ( NOT WIN32 ) + if( NOT WIN32 ) find_library( Log4cxx_LIBRARY log4cxx ) else() find_library( Log4cxx_LIBRARY log4cxx HINTS "C:\\ProgramData\\chocolatey\\lib\\log4cxx\\lib" ) diff --git a/rcl_logging_log4cxx/include/rcl_logging_log4cxx/logging_interface.h b/rcl_logging_log4cxx/include/rcl_logging_log4cxx/logging_interface.h index 01bb280..5260114 100644 --- a/rcl_logging_log4cxx/include/rcl_logging_log4cxx/logging_interface.h +++ b/rcl_logging_log4cxx/include/rcl_logging_log4cxx/logging_interface.h @@ -24,7 +24,9 @@ extern "C" { typedef int rcl_logging_ret_t; RCL_LOGGING_PUBLIC -rcl_logging_ret_t rcl_logging_external_initialize(const char * config_file, rcutils_allocator_t allocator); +rcl_logging_ret_t rcl_logging_external_initialize( + const char * config_file, + rcutils_allocator_t allocator); RCL_LOGGING_PUBLIC rcl_logging_ret_t rcl_logging_external_shutdown(); diff --git a/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp b/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp index 5fc5e59..051edfa 100644 --- a/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp +++ b/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp @@ -12,8 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include -#include +#include +#include +#include +#include +#include #include #include @@ -24,10 +27,10 @@ #include #include -#include -#include -#include -#include +#include +#include +#include +#include /** * Maps the logger name to the log4cxx logger. If the name is null or empty it will map to the @@ -41,32 +44,19 @@ static const log4cxx::LoggerPtr get_logger(const char * name) return log4cxx::Logger::getLogger(name); } -/* These are defined here to match the severity levels in rcl. They provide a consistent way for external logger - implementations to map between the incoming integer severity from ROS to the concept of DEBUG, INFO, WARN, ERROR, - and FATAL*/ -enum RC_LOGGING_LOG_SEVERITY -{ - RC_LOGGING_SEVERITY_UNSET = 0, ///< The unset log level - RC_LOGGING_SEVERITY_DEBUG = 10, ///< The debug log level - RC_LOGGING_SEVERITY_INFO = 20, ///< The info log level - RC_LOGGING_SEVERITY_WARN = 30, ///< The warn log level - RC_LOGGING_SEVERITY_ERROR = 40, ///< The error log level - RC_LOGGING_SEVERITY_FATAL = 50, ///< The fatal log level -}; - static const log4cxx::LevelPtr map_external_log_level_to_library_level(int external_level) { log4cxx::LevelPtr level; // map to the next highest level of severity - if (external_level <= RC_LOGGING_SEVERITY_DEBUG) { + if (external_level <= RCUTILS_LOG_SEVERITY_DEBUG) { level = log4cxx::Level::getDebug(); - } else if (external_level <= RC_LOGGING_SEVERITY_INFO) { + } else if (external_level <= RCUTILS_LOG_SEVERITY_INFO) { level = log4cxx::Level::getInfo(); - } else if (external_level <= RC_LOGGING_SEVERITY_WARN) { + } else if (external_level <= RCUTILS_LOG_SEVERITY_WARN) { level = log4cxx::Level::getWarn(); - } else if (external_level <= RC_LOGGING_SEVERITY_ERROR) { + } else if (external_level <= RCUTILS_LOG_SEVERITY_ERROR) { level = log4cxx::Level::getError(); - } else if (external_level <= RC_LOGGING_SEVERITY_FATAL) { + } else if (external_level <= RCUTILS_LOG_SEVERITY_FATAL) { level = log4cxx::Level::getFatal(); } return level; @@ -78,20 +68,20 @@ extern "C" { #include "rcl_logging_log4cxx/logging_interface.h" -#define RC_LOGGING_RET_OK (0) -#define RC_LOGGING_RET_WARN (1) -#define RC_LOGGING_RET_ERROR (2) -#define RC_LOGGING_RET_INVALID_ARGUMENT (11) -#define RC_LOGGING_RET_CONFIG_FILE_DOESNT_EXIST (21) -#define RC_LOGGING_RET_CONFIG_FILE_INVALID (22) +#define RCL_LOGGING_RET_OK (0) +#define RCL_LOGGING_RET_ERROR (2) +#define RCL_LOGGING_RET_CONFIG_FILE_DOESNT_EXIST (21) +#define RCL_LOGGING_RET_CONFIG_FILE_INVALID (22) -rcl_logging_ret_t rcl_logging_external_initialize(const char * config_file, rcutils_allocator_t allocator) +rcl_logging_ret_t rcl_logging_external_initialize( + const char * config_file, + rcutils_allocator_t allocator) { (void)allocator; bool config_file_provided = (nullptr != config_file) && (config_file[0] != '\0'); bool use_default_config = !config_file_provided; - rcl_logging_ret_t status = RC_LOGGING_RET_OK; + rcl_logging_ret_t status = RCL_LOGGING_RET_OK; if (config_file_provided) { log4cxx::helpers::Pool pool; @@ -99,16 +89,16 @@ rcl_logging_ret_t rcl_logging_external_initialize(const char * config_file, rcut if (!file.exists(pool)) { // The provided config file doesn't exist, fall back to using default configuration use_default_config = true; - status = RC_LOGGING_RET_CONFIG_FILE_DOESNT_EXIST; + status = RCL_LOGGING_RET_CONFIG_FILE_DOESNT_EXIST; } else { - // Attempt to configure the system using the provided config file, but if we fail fall back to using the default - // configuration + // Attempt to configure the system using the provided config file, but if + // we fail fall back to using the default configuration try { log4cxx::PropertyConfigurator::configure(file); } catch (std::exception & ex) { (void)ex; use_default_config = true; - status = RC_LOGGING_RET_CONFIG_FILE_INVALID; + status = RCL_LOGGING_RET_CONFIG_FILE_INVALID; } } } @@ -122,12 +112,12 @@ rcl_logging_ret_t rcl_logging_external_initialize(const char * config_file, rcut // the form ~/.ros/log/__.log // First get the home directory. - const char *homedir = rcutils_get_home_dir(); - if (homedir == NULL) { + const char * homedir = rcutils_get_home_dir(); + if (homedir == nullptr) { // We couldn't get the home directory; it is not really going to be // possible to do logging properly, so get out of here without setting // up logging. - return RC_LOGGING_RET_ERROR; + return RCL_LOGGING_RET_ERROR; } // Now get the milliseconds since the epoch in the local timezone. @@ -136,21 +126,27 @@ rcl_logging_ret_t rcl_logging_external_initialize(const char * config_file, rcut if (ret != RCUTILS_RET_OK) { // We couldn't get the system time, so get out of here without setting up // logging. - return RC_LOGGING_RET_ERROR; + return RCL_LOGGING_RET_ERROR; } int64_t ms_since_epoch = RCUTILS_NS_TO_MS(now); // Get the program name. - char *basec = rcutils_get_executable_name(allocator); - if (basec == NULL) { + char * executable_name = rcutils_get_executable_name(allocator); + if (executable_name == nullptr) { // We couldn't get the program name, so get out of here without setting up // logging. - return RC_LOGGING_RET_ERROR; + return RCL_LOGGING_RET_ERROR; } char log_name_buffer[512] = {0}; - snprintf(log_name_buffer, sizeof(log_name_buffer), "%s/.ros/log/%s_%i_%" PRId64 ".log", homedir, basec, rcutils_get_pid(), ms_since_epoch); - allocator.deallocate(basec, allocator.state); + int print_ret = rcutils_snprintf(log_name_buffer, sizeof(log_name_buffer), + "%s/.ros/log/%s_%i_%" PRId64 ".log", homedir, executable_name, + rcutils_get_pid(), ms_since_epoch); + allocator.deallocate(executable_name, allocator.state); + if (print_ret < 0) { + RCUTILS_SET_ERROR_MSG("Failed to create log file name string"); + return RCL_LOGGING_RET_ERROR; + } std::string log_name_str(log_name_buffer); LOG4CXX_DECODE_CHAR(log_name_l4cxx_str, log_name_str); log4cxx::FileAppenderPtr file_appender(new log4cxx::FileAppender(layout, log_name_l4cxx_str, @@ -164,7 +160,7 @@ rcl_logging_ret_t rcl_logging_external_initialize(const char * config_file, rcut rcl_logging_ret_t rcl_logging_external_shutdown() { log4cxx::BasicConfigurator::resetConfiguration(); - return RC_LOGGING_RET_OK; + return RCL_LOGGING_RET_OK; } void rcl_logging_external_log(int severity, const char * name, const char * msg) @@ -178,7 +174,7 @@ rcl_logging_ret_t rcl_logging_external_set_logger_level(const char * name, int l { log4cxx::LoggerPtr logger(get_logger(name)); logger->setLevel(map_external_log_level_to_library_level(level)); - return RC_LOGGING_RET_OK; + return RCL_LOGGING_RET_OK; } #ifdef __cplusplus diff --git a/rcl_logging_noop/CMakeLists.txt b/rcl_logging_noop/CMakeLists.txt index 2cd514c..36fc66b 100644 --- a/rcl_logging_noop/CMakeLists.txt +++ b/rcl_logging_noop/CMakeLists.txt @@ -37,6 +37,10 @@ ament_target_dependencies(${PROJECT_NAME} rcutils ) +if(BUILD_TESTING) + find_package(ament_lint_auto REQUIRED) + ament_lint_auto_find_test_dependencies() +endif() install(TARGETS ${PROJECT_NAME} ARCHIVE DESTINATION lib diff --git a/rcl_logging_noop/src/rcl_logging_noop/rcl_logging_noop.cpp b/rcl_logging_noop/src/rcl_logging_noop/rcl_logging_noop.cpp index 3acbf32..25a9086 100644 --- a/rcl_logging_noop/src/rcl_logging_noop/rcl_logging_noop.cpp +++ b/rcl_logging_noop/src/rcl_logging_noop/rcl_logging_noop.cpp @@ -12,23 +12,25 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "rcutils/allocator.h" +#include extern "C" { typedef int rcl_logging_ret_t; -#define RC_LOGGING_RET_OK (0) +#define RCL_LOGGING_RET_OK (0) -rcl_logging_ret_t rcl_logging_external_initialize(const char * config_file, rcutils_allocator_t allocator) +rcl_logging_ret_t rcl_logging_external_initialize( + const char * config_file, + rcutils_allocator_t allocator) { (void) config_file; (void) allocator; - return RC_LOGGING_RET_OK; + return RCL_LOGGING_RET_OK; } rcl_logging_ret_t rcl_logging_external_shutdown() { - return RC_LOGGING_RET_OK; + return RCL_LOGGING_RET_OK; } void rcl_logging_external_log(int severity, const char * name, const char * msg) @@ -42,7 +44,7 @@ rcl_logging_ret_t rcl_logging_external_set_logger_level(const char * name, int l { (void) name; (void) level; - return RC_LOGGING_RET_OK; + return RCL_LOGGING_RET_OK; } } /* extern "C" */