Static code review fixes.
authorRomain Forlot <romain.forlot@iot.bzh>
Tue, 23 May 2017 21:59:27 +0000 (23:59 +0200)
committerRomain Forlot <romain.forlot@iot.bzh>
Tue, 23 May 2017 21:59:27 +0000 (23:59 +0200)
Several style errors and mistakes mostly on constructor and passing arguments
as ref.

Change-Id: I2ca921d6aa70b9074392bb7779ade35bebf7bd8d
Signed-off-by: Romain Forlot <romain.forlot@iot.bzh>
18 files changed:
CAN-binder/low-can-binding/binding/configuration.cpp
CAN-binder/low-can-binding/binding/configuration.hpp
CAN-binder/low-can-binding/binding/low-can-cb.cpp
CAN-binder/low-can-binding/can/can-bus.hpp
CAN-binder/low-can-binding/can/can-decoder.cpp
CAN-binder/low-can-binding/can/can-decoder.hpp
CAN-binder/low-can-binding/can/can-message-set.cpp
CAN-binder/low-can-binding/can/can-message-set.hpp
CAN-binder/low-can-binding/can/can-message.cpp
CAN-binder/low-can-binding/can/can-message.hpp
CAN-binder/low-can-binding/can/can-signals.hpp
CAN-binder/low-can-binding/diagnostic/diagnostic-manager.cpp
CAN-binder/low-can-binding/diagnostic/diagnostic-manager.hpp
CAN-binder/low-can-binding/diagnostic/diagnostic-message.cpp
CAN-binder/low-can-binding/diagnostic/diagnostic-message.hpp
CAN-binder/low-can-binding/utils/config-parser.hpp
CAN-binder/low-can-binding/utils/timer.cpp
CAN-binder/low-can-binding/utils/timer.hpp

index 623ba78..8e6ddde 100644 (file)
@@ -78,7 +78,7 @@ void configuration_t::set_active_message_set(uint8_t id)
 }
 
 
-std::shared_ptr<diagnostic_message_t> configuration_t::get_diagnostic_message(std::string message_name) const
+std::shared_ptr<diagnostic_message_t> 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<diagnostic_message_t> 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<diagnostic_message_t> diag_msg = get_diagnostic_message(message_name);
        if( diag_msg != nullptr && diag_msg->get_supported())
index 3ab88ee..fc53d38 100644 (file)
@@ -81,8 +81,8 @@ class configuration_t
 
                void set_active_message_set(uint8_t id);
 
-               std::shared_ptr<diagnostic_message_t> get_diagnostic_message(std::string message_name) const;
-               DiagnosticRequest* get_request_from_diagnostic_message(std::string message_name) const;
+               std::shared_ptr<diagnostic_message_t> 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
index 9aa2340..9e85f5c 100644 (file)
@@ -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<std::mutex> 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<std::string, struct event_filter_t> 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)
index 1efd204..16d8cf6 100644 (file)
@@ -71,7 +71,7 @@ private:
 
        std::vector<std::pair<std::string, std::string> > 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();
index aa1f57e..3e21397 100644 (file)
@@ -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<std::shared_ptr<can_signal_t> > signals, float value, bool* send)
+               const std::vector<std::shared_ptr<can_signal_t> >& 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<std::shared_ptr<can_signal_t> > signals, float value, bool* send)
+               const std::vector<std::shared_ptr<can_signal_t> >& 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<std::shared_ptr<can_signal_t> > signals, float value, bool* send)
+               const std::vector<std::shared_ptr<can_signal_t> >& 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<std::shared_ptr<can_signal_t> > signals, float value, bool* send)
+               const std::vector<std::shared_ptr<can_signal_t> >& 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<std::shared_ptr<can_signal_t> > signals, bool* send)
+       const std::vector<std::shared_ptr<can_signal_t> >& 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<std::shared_ptr<can_signal_t> > signals, bool* send)
+               float value, const std::vector<std::shared_ptr<can_signal_t> >& 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<std::shared_ptr<can_signal_t> > signals, bool* send)
+               const can_message_t& message, const std::vector<std::shared_ptr<can_signal_t> >& signals, bool* send)
 {
        float value = parseSignalBitfield(signal, message);
        return decodeSignal(signal, value, signals, send);
index 238bfea..7eaa4a7 100644 (file)
@@ -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<std::shared_ptr<can_signal_t> > signals,
+       static openxc_DynamicField stateDecoder(can_signal_t& signal, const std::vector<std::shared_ptr<can_signal_t> >& signals,
                        float value, bool* send);
-       static openxc_DynamicField booleanDecoder(can_signal_t& signal, const std::vector<std::shared_ptr<can_signal_t> > signals,
+       static openxc_DynamicField booleanDecoder(can_signal_t& signal, const std::vector<std::shared_ptr<can_signal_t> >& signals,
                        float value, bool* send);
-       static openxc_DynamicField ignoreDecoder(can_signal_t& signal, const std::vector<std::shared_ptr<can_signal_t> > signals,
+       static openxc_DynamicField ignoreDecoder(can_signal_t& signal, const std::vector<std::shared_ptr<can_signal_t> >& signals,
                        float value, bool* send);
-       static openxc_DynamicField noopDecoder(can_signal_t& signal, const std::vector<std::shared_ptr<can_signal_t> > signals,
+       static openxc_DynamicField noopDecoder(can_signal_t& signal, const std::vector<std::shared_ptr<can_signal_t> >& signals,
                        float value, bool* send);
 
        static openxc_DynamicField translateSignal(can_signal_t& signal, const can_message_t& message,
-               const std::vector<std::shared_ptr<can_signal_t> > signals, bool* send);
+               const std::vector<std::shared_ptr<can_signal_t> >& signals, bool* send);
 
        static openxc_DynamicField decodeSignal(can_signal_t& signal, const can_message_t& message,
-                       const std::vector<std::shared_ptr<can_signal_t> > signals, bool* send);
+                       const std::vector<std::shared_ptr<can_signal_t> >& signals, bool* send);
 
        static openxc_DynamicField decodeSignal(can_signal_t& signal, float value,
-               const std::vector<std::shared_ptr<can_signal_t> > signals, bool* send);
+               const std::vector<std::shared_ptr<can_signal_t> >& signals, bool* send);
 
        static float decode_obd2_response(const DiagnosticResponse* response, float parsed_payload);
 
index 5b3a220..a41f008 100644 (file)
@@ -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<std::shared_ptr<can_message_definition_t> >& can_messages_definition,
                const std::vector<std::shared_ptr<diagnostic_message_t> >& diagnostic_messages)
        : index_{index}
index ca01abc..c763de6 100644 (file)
@@ -40,7 +40,7 @@ private:
 public:
        can_message_set_t(
                        uint8_t index,
-                       const std::string name,
+                       const std::string& name,
                        const std::vector<std::shared_ptr<can_message_definition_t> >& can_messages_definition,
                        const std::vector<std::shared_ptr<diagnostic_message_t> >& diagnostic_messages);
 
index 772734d..abfbfdb 100644 (file)
@@ -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<uint8_t> data,
+       std::vector<uint8_t>& data,
        uint64_t timestamp)
        :  maxdlen_{maxdlen},
        id_{id},
index 910b3e9..1ce1c7a 100644 (file)
@@ -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<uint8_t> 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<uint8_t>& data, uint64_t timestamp);
 
        uint32_t get_id() const;
        bool get_rtr_flag_() const;
index cc4b6da..b8fbe4e 100644 (file)
@@ -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<std::shared_ptr<can_signal_t> > signals, float value, bool* send);
+               const std::vector<std::shared_ptr<can_signal_t> >& signals, float value, bool* send);
 
 ///
 /// @brief: The type signature for a CAN signal encoder.
index 199054b..4f4b842 100644 (file)
@@ -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)
 {
index 9f19b01..d942b47 100644 (file)
@@ -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);
index 4455aa8..df093b0 100644 (file)
@@ -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}
 {}
 
index ac427b9..65ed9c0 100644 (file)
@@ -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();
index 0453110..3115e9b 100644 (file)
@@ -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();
index 9c22bd0..4584b85 100644 (file)
@@ -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;
index 432d513..560a818 100644 (file)
@@ -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;