From 7747851ca010a3dfe9ffee808376dd5a7af68b91 Mon Sep 17 00:00:00 2001 From: Romain Forlot Date: Tue, 23 May 2017 23:59:27 +0200 Subject: [PATCH] Static code review fixes. Several style errors and mistakes mostly on constructor and passing arguments as ref. Change-Id: I2ca921d6aa70b9074392bb7779ade35bebf7bd8d Signed-off-by: Romain Forlot --- CAN-binder/low-can-binding/binding/configuration.cpp | 4 ++-- CAN-binder/low-can-binding/binding/configuration.hpp | 4 ++-- CAN-binder/low-can-binding/binding/low-can-cb.cpp | 7 +++++-- CAN-binder/low-can-binding/can/can-bus.hpp | 2 +- CAN-binder/low-can-binding/can/can-decoder.cpp | 16 ++++++++-------- CAN-binder/low-can-binding/can/can-decoder.hpp | 14 +++++++------- CAN-binder/low-can-binding/can/can-message-set.cpp | 2 +- CAN-binder/low-can-binding/can/can-message-set.hpp | 2 +- CAN-binder/low-can-binding/can/can-message.cpp | 2 +- CAN-binder/low-can-binding/can/can-message.hpp | 2 +- CAN-binder/low-can-binding/can/can-signals.hpp | 2 +- .../low-can-binding/diagnostic/diagnostic-manager.cpp | 10 ++++------ .../low-can-binding/diagnostic/diagnostic-manager.hpp | 2 +- .../low-can-binding/diagnostic/diagnostic-message.cpp | 4 ++-- .../low-can-binding/diagnostic/diagnostic-message.hpp | 2 +- CAN-binder/low-can-binding/utils/config-parser.hpp | 2 +- CAN-binder/low-can-binding/utils/timer.cpp | 6 +++--- CAN-binder/low-can-binding/utils/timer.hpp | 2 +- 18 files changed, 43 insertions(+), 42 deletions(-) diff --git a/CAN-binder/low-can-binding/binding/configuration.cpp b/CAN-binder/low-can-binding/binding/configuration.cpp index 623ba78..8e6ddde 100644 --- a/CAN-binder/low-can-binding/binding/configuration.cpp +++ b/CAN-binder/low-can-binding/binding/configuration.cpp @@ -78,7 +78,7 @@ void configuration_t::set_active_message_set(uint8_t id) } -std::shared_ptr configuration_t::get_diagnostic_message(std::string message_name) const +std::shared_ptr configuration_t::get_diagnostic_message(const std::string& message_name) const { struct utils::signals_found found; found = utils::signals_manager_t::instance().find_signals(build_DynamicField(message_name)); @@ -87,7 +87,7 @@ std::shared_ptr configuration_t::get_diagnostic_message(st return nullptr; } -DiagnosticRequest* configuration_t::get_request_from_diagnostic_message(std::string message_name) const +DiagnosticRequest* configuration_t::get_request_from_diagnostic_message(const std::string& message_name) const { std::shared_ptr diag_msg = get_diagnostic_message(message_name); if( diag_msg != nullptr && diag_msg->get_supported()) diff --git a/CAN-binder/low-can-binding/binding/configuration.hpp b/CAN-binder/low-can-binding/binding/configuration.hpp index 3ab88ee..fc53d38 100644 --- a/CAN-binder/low-can-binding/binding/configuration.hpp +++ b/CAN-binder/low-can-binding/binding/configuration.hpp @@ -81,8 +81,8 @@ class configuration_t void set_active_message_set(uint8_t id); - std::shared_ptr get_diagnostic_message(std::string message_name) const; - DiagnosticRequest* get_request_from_diagnostic_message(std::string message_name) const; + std::shared_ptr get_diagnostic_message(const std::string& message_name) const; + DiagnosticRequest* get_request_from_diagnostic_message(const std::string& message_name) const; /* /// TODO: implement this function as method into can_bus class /// @brief Pre initialize actions made before CAN bus initialization diff --git a/CAN-binder/low-can-binding/binding/low-can-cb.cpp b/CAN-binder/low-can-binding/binding/low-can-cb.cpp index 9aa2340..9e85f5c 100644 --- a/CAN-binder/low-can-binding/binding/low-can-cb.cpp +++ b/CAN-binder/low-can-binding/binding/low-can-cb.cpp @@ -52,7 +52,7 @@ void on_no_clients(std::string message) } } -static void push_n_notify(const can_message_t cm) +static void push_n_notify(const can_message_t& cm) { can_bus_t& cbm = configuration_t::instance().get_can_bus_manager(); std::lock_guard can_message_lock(cbm.get_can_message_mutex()); @@ -181,7 +181,7 @@ static int subscribe_unsubscribe_signal(struct afb_req request, bool subscribe, /// static int subscribe_unsubscribe_signals(struct afb_req request, bool subscribe, const struct utils::signals_found& signals) { - int ret, rets = 0; + int rets = 0; //TODO: Implement way to dynamically call the right function no matter // how much signals types we have. @@ -189,6 +189,7 @@ static int subscribe_unsubscribe_signals(struct afb_req request, bool subscribe, for(const auto& sig : signals.diagnostic_messages) { + int ret = 0; diagnostic_manager_t& diag_m = conf.get_diagnostic_manager(); DiagnosticRequest* diag_req = conf.get_request_from_diagnostic_message(sig->get_name()); @@ -295,6 +296,8 @@ static const std::map parse_args_from_reques args = afb_req_json(request); if (args == NULL || !json_object_object_get_ex(args, "event", &event)) { + event = json_object_new_string("*"); + parse_filter(event, &event_filter); ret["*"] = event_filter; } else if (json_object_get_type(event) != json_type_array) diff --git a/CAN-binder/low-can-binding/can/can-bus.hpp b/CAN-binder/low-can-binding/can/can-bus.hpp index 1efd204..16d8cf6 100644 --- a/CAN-binder/low-can-binding/can/can-bus.hpp +++ b/CAN-binder/low-can-binding/can/can-bus.hpp @@ -71,7 +71,7 @@ private: std::vector > can_devices_; public: - can_bus_t(utils::config_parser_t conf_file); + explicit can_bus_t(utils::config_parser_t conf_file); can_bus_t(can_bus_t&&); void set_can_devices(); diff --git a/CAN-binder/low-can-binding/can/can-decoder.cpp b/CAN-binder/low-can-binding/can/can-decoder.cpp index aa1f57e..3e21397 100644 --- a/CAN-binder/low-can-binding/can/can-decoder.cpp +++ b/CAN-binder/low-can-binding/can/can-decoder.cpp @@ -54,7 +54,7 @@ float decoder_t::parseSignalBitfield(can_signal_t& signal, const can_message_t& /// always succeeds. /// openxc_DynamicField decoder_t::noopDecoder(can_signal_t& signal, - const std::vector > signals, float value, bool* send) + const std::vector >& signals, float value, bool* send) { openxc_DynamicField decoded_value = build_DynamicField(value); @@ -76,7 +76,7 @@ openxc_DynamicField decoder_t::noopDecoder(can_signal_t& signal, /// decoder always succeeds. /// openxc_DynamicField decoder_t::booleanDecoder(can_signal_t& signal, - const std::vector > signals, float value, bool* send) + const std::vector >& signals, float value, bool* send) { openxc_DynamicField decoded_value = build_DynamicField(value == 0.0 ? false : true); @@ -98,7 +98,7 @@ openxc_DynamicField decoder_t::booleanDecoder(can_signal_t& signal, /// @return Return value is undefined. /// openxc_DynamicField decoder_t::ignoreDecoder(can_signal_t& signal, - const std::vector > signals, float value, bool* send) + const std::vector >& signals, float value, bool* send) { if(send) *send = false; @@ -125,7 +125,7 @@ openxc_DynamicField decoder_t::ignoreDecoder(can_signal_t& signal, /// return value is undefined. /// openxc_DynamicField decoder_t::stateDecoder(can_signal_t& signal, - const std::vector > signals, float value, bool* send) + const std::vector >& signals, float value, bool* send) { const std::string signal_state = signal.get_states((uint8_t)value); openxc_DynamicField decoded_value = build_DynamicField(signal_state); @@ -154,7 +154,7 @@ openxc_DynamicField decoder_t::stateDecoder(can_signal_t& signal, /// string or boolean. /// openxc_DynamicField decoder_t::translateSignal(can_signal_t& signal, const can_message_t& message, - const std::vector > signals, bool* send) + const std::vector >& signals, bool* send) { float value = decoder_t::parseSignalBitfield(signal, message); DEBUG(binder_interface, "%s: Decoded message from parseSignalBitfield: %f", __FUNCTION__, value); @@ -167,7 +167,7 @@ openxc_DynamicField decoder_t::translateSignal(can_signal_t& signal, const can_m signal.set_received(true); // Don't send if they is no changes - if ((signal.get_last_value() == value && !signal.get_send_same()) || !send ) + if ((signal.get_last_value() == value && !signal.get_send_same()) || !*send ) { *send = false; } @@ -193,7 +193,7 @@ openxc_DynamicField decoder_t::translateSignal(can_signal_t& signal, const can_m /// string or boolean. If 'send' is false, the return value is undefined. /// openxc_DynamicField decoder_t::decodeSignal( can_signal_t& signal, - float value, const std::vector > signals, bool* send) + float value, const std::vector >& signals, bool* send) { SignalDecoder decoder = signal.get_decoder() == nullptr ? noopDecoder : signal.get_decoder(); @@ -216,7 +216,7 @@ openxc_DynamicField decoder_t::decodeSignal( can_signal_t& signal, /// not be decoded. /// openxc_DynamicField decoder_t::decodeSignal( can_signal_t& signal, - const can_message_t& message, const std::vector > signals, bool* send) + const can_message_t& message, const std::vector >& signals, bool* send) { float value = parseSignalBitfield(signal, message); return decodeSignal(signal, value, signals, send); diff --git a/CAN-binder/low-can-binding/can/can-decoder.hpp b/CAN-binder/low-can-binding/can/can-decoder.hpp index 238bfea..7eaa4a7 100644 --- a/CAN-binder/low-can-binding/can/can-decoder.hpp +++ b/CAN-binder/low-can-binding/can/can-decoder.hpp @@ -26,23 +26,23 @@ class decoder_t public: static float parseSignalBitfield(can_signal_t& signal, const can_message_t& message); - static openxc_DynamicField stateDecoder(can_signal_t& signal, const std::vector > signals, + static openxc_DynamicField stateDecoder(can_signal_t& signal, const std::vector >& signals, float value, bool* send); - static openxc_DynamicField booleanDecoder(can_signal_t& signal, const std::vector > signals, + static openxc_DynamicField booleanDecoder(can_signal_t& signal, const std::vector >& signals, float value, bool* send); - static openxc_DynamicField ignoreDecoder(can_signal_t& signal, const std::vector > signals, + static openxc_DynamicField ignoreDecoder(can_signal_t& signal, const std::vector >& signals, float value, bool* send); - static openxc_DynamicField noopDecoder(can_signal_t& signal, const std::vector > signals, + static openxc_DynamicField noopDecoder(can_signal_t& signal, const std::vector >& signals, float value, bool* send); static openxc_DynamicField translateSignal(can_signal_t& signal, const can_message_t& message, - const std::vector > signals, bool* send); + const std::vector >& signals, bool* send); static openxc_DynamicField decodeSignal(can_signal_t& signal, const can_message_t& message, - const std::vector > signals, bool* send); + const std::vector >& signals, bool* send); static openxc_DynamicField decodeSignal(can_signal_t& signal, float value, - const std::vector > signals, bool* send); + const std::vector >& signals, bool* send); static float decode_obd2_response(const DiagnosticResponse* response, float parsed_payload); diff --git a/CAN-binder/low-can-binding/can/can-message-set.cpp b/CAN-binder/low-can-binding/can/can-message-set.cpp index 5b3a220..a41f008 100644 --- a/CAN-binder/low-can-binding/can/can-message-set.cpp +++ b/CAN-binder/low-can-binding/can/can-message-set.cpp @@ -22,7 +22,7 @@ can_message_set_t::can_message_set_t( uint8_t index, - const std::string name, + const std::string& name, const std::vector >& can_messages_definition, const std::vector >& diagnostic_messages) : index_{index} diff --git a/CAN-binder/low-can-binding/can/can-message-set.hpp b/CAN-binder/low-can-binding/can/can-message-set.hpp index ca01abc..c763de6 100644 --- a/CAN-binder/low-can-binding/can/can-message-set.hpp +++ b/CAN-binder/low-can-binding/can/can-message-set.hpp @@ -40,7 +40,7 @@ private: public: can_message_set_t( uint8_t index, - const std::string name, + const std::string& name, const std::vector >& can_messages_definition, const std::vector >& diagnostic_messages); diff --git a/CAN-binder/low-can-binding/can/can-message.cpp b/CAN-binder/low-can-binding/can/can-message.cpp index 772734d..abfbfdb 100644 --- a/CAN-binder/low-can-binding/can/can-message.cpp +++ b/CAN-binder/low-can-binding/can/can-message.cpp @@ -36,7 +36,7 @@ can_message_t::can_message_t(uint8_t maxdlen, can_message_format_t format, bool rtr_flag, uint8_t flags, - std::vector data, + std::vector& data, uint64_t timestamp) : maxdlen_{maxdlen}, id_{id}, diff --git a/CAN-binder/low-can-binding/can/can-message.hpp b/CAN-binder/low-can-binding/can/can-message.hpp index 910b3e9..1ce1c7a 100644 --- a/CAN-binder/low-can-binding/can/can-message.hpp +++ b/CAN-binder/low-can-binding/can/can-message.hpp @@ -55,7 +55,7 @@ private: public: can_message_t(); - can_message_t(uint8_t maxdlen, uint32_t id, uint8_t length, can_message_format_t format, bool rtr_flag_, uint8_t flags, std::vector data, uint64_t timestamp); + can_message_t(uint8_t maxdlen, uint32_t id, uint8_t length, can_message_format_t format, bool rtr_flag_, uint8_t flags, std::vector& data, uint64_t timestamp); uint32_t get_id() const; bool get_rtr_flag_() const; diff --git a/CAN-binder/low-can-binding/can/can-signals.hpp b/CAN-binder/low-can-binding/can/can-signals.hpp index cc4b6da..b8fbe4e 100644 --- a/CAN-binder/low-can-binding/can/can-signals.hpp +++ b/CAN-binder/low-can-binding/can/can-signals.hpp @@ -56,7 +56,7 @@ class can_signal_t; /// @return a decoded value in an openxc_DynamicField struct. /// typedef openxc_DynamicField (*SignalDecoder)(can_signal_t& signal, - const std::vector > signals, float value, bool* send); + const std::vector >& signals, float value, bool* send); /// /// @brief: The type signature for a CAN signal encoder. diff --git a/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.cpp b/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.cpp index 199054b..4f4b842 100644 --- a/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.cpp +++ b/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.cpp @@ -33,7 +33,7 @@ #define MICRO 1000000 diagnostic_manager_t::diagnostic_manager_t() - : initialized_{false} + : initialized_{false}, event_source_{nullptr} {} /// @brief Diagnostic manager isn't initialized at launch but after @@ -185,6 +185,7 @@ void diagnostic_manager_t::shims_logger(const char* format, ...) vsnprintf(buffer, 256, format, args); DEBUG(binder_interface, "%s: %s", __FUNCTION__, buffer); + va_end(args); } /// @brief The type signature for a... OpenXC TODO: not used yet. @@ -288,10 +289,7 @@ active_diagnostic_request_t* diagnostic_manager_t::find_recurring_request(const if(entry != nullptr) { if(diagnostic_request_equals(&entry->get_handle()->request, request)) - { - return entry; - break; - } + {return entry;} } } return nullptr; @@ -323,7 +321,7 @@ active_diagnostic_request_t* diagnostic_manager_t::find_recurring_request(const /// @return true if the request was added successfully. Returns false if there /// wasn't a free active request entry, if the frequency was too high or if the /// CAN acceptance filters could not be configured, -active_diagnostic_request_t* diagnostic_manager_t::add_request(DiagnosticRequest* request, const std::string name, +active_diagnostic_request_t* diagnostic_manager_t::add_request(DiagnosticRequest* request, const std::string& name, bool wait_for_multiple_responses, const DiagnosticResponseDecoder decoder, const DiagnosticResponseCallback callback) { diff --git a/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.hpp b/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.hpp index 9f19b01..d942b47 100644 --- a/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.hpp +++ b/CAN-binder/low-can-binding/diagnostic/diagnostic-manager.hpp @@ -79,7 +79,7 @@ public: active_diagnostic_request_t* find_recurring_request(const DiagnosticRequest* request); // Subscription parts - active_diagnostic_request_t* add_request(DiagnosticRequest* request, const std::string name, + active_diagnostic_request_t* add_request(DiagnosticRequest* request, const std::string& name, bool waitForMultipleResponses, const DiagnosticResponseDecoder decoder, const DiagnosticResponseCallback callback); bool validate_optional_request_attributes(float frequencyHz); diff --git a/CAN-binder/low-can-binding/diagnostic/diagnostic-message.cpp b/CAN-binder/low-can-binding/diagnostic/diagnostic-message.cpp index 4455aa8..df093b0 100644 --- a/CAN-binder/low-can-binding/diagnostic/diagnostic-message.cpp +++ b/CAN-binder/low-can-binding/diagnostic/diagnostic-message.cpp @@ -34,10 +34,10 @@ const char *UNIT_NAMES[10] = { "NM" }; -diagnostic_message_t::diagnostic_message_t(uint8_t pid, const std::string generic_name, const int min, +diagnostic_message_t::diagnostic_message_t(uint8_t pid, const std::string& generic_name, const int min, const int max, enum UNIT unit, float frequency, DiagnosticResponseDecoder decoder, DiagnosticResponseCallback callback, bool supported) - : pid_{pid}, generic_name_{generic_name}, min_{min}, max_{max}, unit_{unit}, + : parent_{nullptr}, pid_{pid}, generic_name_{generic_name}, min_{min}, max_{max}, unit_{unit}, frequency_{frequency}, decoder_{decoder}, callback_{callback}, supported_{supported} {} diff --git a/CAN-binder/low-can-binding/diagnostic/diagnostic-message.hpp b/CAN-binder/low-can-binding/diagnostic/diagnostic-message.hpp index ac427b9..65ed9c0 100644 --- a/CAN-binder/low-can-binding/diagnostic/diagnostic-message.hpp +++ b/CAN-binder/low-can-binding/diagnostic/diagnostic-message.hpp @@ -64,7 +64,7 @@ class diagnostic_message_t public: const char* generic_name = generic_name_.c_str(); - diagnostic_message_t(uint8_t pid, const std::string generic_name, const int min, const int max, enum UNIT unit, float frequency, + diagnostic_message_t(uint8_t pid, const std::string& generic_name, const int min, const int max, enum UNIT unit, float frequency, DiagnosticResponseDecoder decoder, DiagnosticResponseCallback callback, bool supported); uint32_t get_pid(); diff --git a/CAN-binder/low-can-binding/utils/config-parser.hpp b/CAN-binder/low-can-binding/utils/config-parser.hpp index 0453110..3115e9b 100644 --- a/CAN-binder/low-can-binding/utils/config-parser.hpp +++ b/CAN-binder/low-can-binding/utils/config-parser.hpp @@ -35,7 +35,7 @@ namespace utils public: config_parser_t(config_parser_t&&) = default; config_parser_t(const config_parser_t&) = default; - config_parser_t(std::string conf_file); + explicit config_parser_t(std::string conf_file); const std::string& filepath() const; bool check_conf(); diff --git a/CAN-binder/low-can-binding/utils/timer.cpp b/CAN-binder/low-can-binding/utils/timer.cpp index 9c22bd0..4584b85 100644 --- a/CAN-binder/low-can-binding/utils/timer.cpp +++ b/CAN-binder/low-can-binding/utils/timer.cpp @@ -24,7 +24,7 @@ long long int system_time_us() { struct timespec t_usec; - long long int timestamp_usec; + long long int timestamp_usec = 0; if(!::clock_gettime(CLOCK_MONOTONIC, &t_usec)) timestamp_usec = (t_usec.tv_nsec / 1000ll) + (t_usec.tv_sec* 1000000ll); @@ -34,7 +34,7 @@ long long int system_time_us() long long int system_time_ms() { struct timespec t_msec; - long long int timestamp_msec; + long long int timestamp_msec = 0; if(!::clock_gettime(CLOCK_MONOTONIC, &t_msec)) timestamp_msec = (t_msec.tv_nsec / 1000000ll) + (t_msec.tv_sec* 1000ll); @@ -44,7 +44,7 @@ long long int system_time_ms() long long int system_time_s() { struct timespec t_sec; - long long int timestamp_sec; + long long int timestamp_sec = 0; if(!::clock_gettime(CLOCK_MONOTONIC, &t_sec)) timestamp_sec = t_sec.tv_sec; diff --git a/CAN-binder/low-can-binding/utils/timer.hpp b/CAN-binder/low-can-binding/utils/timer.hpp index 432d513..560a818 100644 --- a/CAN-binder/low-can-binding/utils/timer.hpp +++ b/CAN-binder/low-can-binding/utils/timer.hpp @@ -42,7 +42,7 @@ private: public: frequency_clock_t(); - frequency_clock_t(float frequency); + explicit frequency_clock_t(float frequency); frequency_clock_t(float frequency, uint64_t last_tick, time_function_t time_function); float get_frequency() const; -- 2.16.6