Skip to content

Commit b1e7bab

Browse files
kheaactuaDuarte Fonseca
andauthored
Code Quality: Address shadowing (#840)
Summary Code Quality: Address shadowing Details Addresses most shadowing variables Fixes some old style C-casts Fixed static_cast and scoped_locks Co-authored-by: Duarte Fonseca <[email protected]>
1 parent 3d9972e commit b1e7bab

16 files changed

+321
-324
lines changed

implementation/configuration/src/configuration_impl.cpp

Lines changed: 39 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2283,24 +2283,23 @@ void configuration_impl::load_eventgroup(
22832283
}
22842284
its_converter >> its_eventgroup->id_;
22852285
} else if (its_key == "is_multicast") {
2286-
std::string its_value(j->second.data());
2287-
if (its_value == "true") {
2286+
std::string its_value_inner(j->second.data());
2287+
if (its_value_inner == "true") {
22882288
its_eventgroup->multicast_address_ = _service->multicast_address_;
22892289
its_eventgroup->multicast_port_ = _service->multicast_port_;
22902290
}
22912291
} else if (its_key == "multicast") {
22922292
try {
2293-
std::string its_value = j->second.get_child("address").data();
2294-
its_eventgroup->multicast_address_ = its_value;
2295-
its_value = j->second.get_child("port").data();
2296-
its_converter << its_value;
2293+
std::string its_value_inner = j->second.get_child("address").data();
2294+
its_eventgroup->multicast_address_ = its_value_inner;
2295+
its_value_inner = j->second.get_child("port").data();
2296+
its_converter << its_value_inner;
22972297
its_converter >> its_eventgroup->multicast_port_;
22982298
} catch (...) {
22992299
// intentionally left empty
23002300
}
23012301
} else if (its_key == "threshold") {
23022302
int its_threshold(0);
2303-
std::stringstream its_converter;
23042303
its_converter << std::dec << its_value;
23052304
its_converter >> std::dec >> its_threshold;
23062305
its_eventgroup->threshold_ =
@@ -2309,13 +2308,12 @@ void configuration_impl::load_eventgroup(
23092308
static_cast<uint8_t>(its_threshold);
23102309
} else if (its_key == "events") {
23112310
for (auto k = j->second.begin(); k != j->second.end(); ++k) {
2312-
std::stringstream its_converter;
2313-
std::string its_value(k->second.data());
2311+
std::string its_value_inner(k->second.data());
23142312
event_t its_event_id(0);
2315-
if (its_value.size() > 1 && its_value[0] == '0' && its_value[1] == 'x') {
2316-
its_converter << std::hex << its_value;
2313+
if (its_value_inner.size() > 1 && its_value_inner[0] == '0' && its_value_inner[1] == 'x') {
2314+
its_converter << std::hex << its_value_inner;
23172315
} else {
2318-
its_converter << std::dec << its_value;
2316+
its_converter << std::dec << its_value_inner;
23192317
}
23202318
its_converter >> its_event_id;
23212319
if (0 < its_event_id) {
@@ -3565,7 +3563,6 @@ bool configuration_impl::find_specific_port(uint16_t &_port, service_t _service,
35653563
std::map<bool, std::set<uint16_t> > &_used_client_ports) const {
35663564
bool is_configured(false);
35673565
bool check_all(false);
3568-
std::list<std::shared_ptr<client>>::const_iterator it;
35693566
auto its_client = find_client(_service, _instance);
35703567

35713568
// Check for service, instance specific port configuration
@@ -3594,7 +3591,7 @@ bool configuration_impl::find_specific_port(uint16_t &_port, service_t _service,
35943591
<< " service: " << std::hex << _service
35953592
<< " instance: " << _instance
35963593
<< " reliable: " << std::dec << _reliable
3597-
<< " return specific port: " << (uint32_t)_port;
3594+
<< " return specific port: " << static_cast<uint32_t>(_port);
35983595
return true;
35993596
}
36003597
++it;
@@ -3611,7 +3608,7 @@ bool configuration_impl::find_specific_port(uint16_t &_port, service_t _service,
36113608
<< " service: " << std::hex << _service
36123609
<< " instance: " << _instance
36133610
<< " reliable: " << std::dec << _reliable
3614-
<< " return specific port: " << (uint32_t)_port;
3611+
<< " return specific port: " << static_cast<uint32_t>(_port);
36153612
return true;
36163613
}
36173614
}
@@ -4395,25 +4392,25 @@ void configuration_impl::load_acceptance_data(const boost::property_tree::ptree&
43954392
port_type_e its_type(port_type_e::PT_OPTIONAL);
43964393

43974394
for (const auto& range : p.second) {
4398-
const std::string its_key(range.first);
4399-
const std::string its_value(range.second.data());
4400-
if (its_key == "first" || its_key == "last" || its_key == "port") {
4401-
its_converter << std::dec << its_value;
4395+
const std::string its_key_inner(range.first);
4396+
const std::string its_value_inner(range.second.data());
4397+
if (its_key_inner == "first" || its_key_inner == "last" || its_key_inner == "port") {
4398+
its_converter << std::dec << its_value_inner;
44024399
std::uint16_t its_port_value(0);
44034400
its_converter >> its_port_value;
44044401
its_converter.str("");
44054402
its_converter.clear();
4406-
if (its_key == "first") {
4403+
if (its_key_inner == "first") {
44074404
its_first = its_port_value;
4408-
} else if (its_key == "last") {
4405+
} else if (its_key_inner == "last") {
44094406
its_last = its_port_value;
4410-
} else if (its_key == "port") {
4407+
} else if (its_key_inner == "port") {
44114408
its_first = its_last = its_port_value;
44124409
}
4413-
} else if (its_key == "type") {
4414-
if (its_value == "secure") {
4410+
} else if (its_key_inner == "type") {
4411+
if (its_value_inner == "secure") {
44154412
its_type = port_type_e::PT_SECURE;
4416-
} else if (its_value == "optional") {
4413+
} else if (its_value_inner == "optional") {
44174414
its_type = port_type_e::PT_OPTIONAL;
44184415
} else {
44194416
its_type = port_type_e::PT_UNKNOWN;
@@ -4604,17 +4601,17 @@ void configuration_impl::load_someip_tp_for_service(
46044601
const std::string its_value(method.second.data());
46054602
if (its_value.empty()) {
46064603
for (const auto &its_data : method.second) {
4607-
const std::string its_value(its_data.second.data());
4608-
if (!its_value.empty()) {
4604+
const std::string its_value_inner(its_data.second.data());
4605+
if (!its_value_inner.empty()) {
46094606
if (its_data.first == "method") {
4610-
if (its_value.size() > 1 && its_value[0] == '0' && its_value[1] == 'x') {
4611-
its_converter << std::hex << its_value;
4607+
if (its_value_inner.size() > 1 && its_value_inner[0] == '0' && its_value_inner[1] == 'x') {
4608+
its_converter << std::hex << its_value_inner;
46124609
} else {
4613-
its_converter << std::dec << its_value;
4610+
its_converter << std::dec << its_value_inner;
46144611
}
46154612
its_converter >> its_method;
46164613
} else if (its_data.first == "max-segment-length") {
4617-
its_converter << std::dec << its_value;
4614+
its_converter << std::dec << its_value_inner;
46184615
its_converter >> its_max_segment_length;
46194616

46204617
// Segment length must be multiple of 16
@@ -4628,7 +4625,7 @@ void configuration_impl::load_someip_tp_for_service(
46284625
its_max_segment_length = std::uint16_t(its_max_segment_length - its_rest);
46294626
}
46304627
} else if (its_data.first == "separation-time") {
4631-
its_converter << std::dec << its_value;
4628+
its_converter << std::dec << its_value_inner;
46324629
its_converter >> its_separation_time;
46334630
its_separation_time *= std::uint32_t(1000);
46344631
}
@@ -4998,11 +4995,11 @@ void configuration_impl::set_sd_acceptance_rule(
49984995
sd_acceptance_rules_active_.insert(_address);
49994996
}
50004997

5001-
const auto found_reliability = found_address->second.second.find(_reliable);
4998+
const auto found_reliability_inner = found_address->second.second.find(_reliable);
50024999
VSOMEIP_INFO << "ipsec:acceptance:" << _address << "[" << _path << "]"
50035000
<< ":" << (_reliable ? "tcp" : "udp") << ": using default ranges "
5004-
<< found_reliability->second.first << " "
5005-
<< found_reliability->second.second;
5001+
<< found_reliability_inner->second.first << " "
5002+
<< found_reliability_inner->second.second;
50065003
}
50075004
} else if (_enable) {
50085005
boost::icl::interval_set<std::uint16_t> its_optional_default;
@@ -5034,14 +5031,14 @@ void configuration_impl::set_sd_acceptance_rule(
50345031
sd_acceptance_rules_active_.insert(_address);
50355032
}
50365033

5037-
const auto found_address = sd_acceptance_rules_.find(_address);
5038-
if (found_address != sd_acceptance_rules_.end()) {
5039-
const auto found_reliability = found_address->second.second.find(_reliable);
5040-
if (found_reliability != found_address->second.second.end()) {
5041-
VSOMEIP_INFO << "ipsec:acceptance:" << _address << "[" << _path << "]"
5034+
const auto found_address_inner = sd_acceptance_rules_.find(_address);
5035+
if (found_address_inner != sd_acceptance_rules_.end()) {
5036+
const auto found_reliability_inner = found_address_inner->second.second.find(_reliable);
5037+
if (found_reliability_inner != found_address_inner->second.second.end()) {
5038+
VSOMEIP_INFO << "ipsec:acceptance:" << _address
50425039
<< ":" << (_reliable ? "tcp" : "udp") << ": using default ranges "
5043-
<< found_reliability->second.first << " "
5044-
<< found_reliability->second.second;
5040+
<< found_reliability_inner->second.first << " "
5041+
<< found_reliability_inner->second.second;
50455042
}
50465043
}
50475044
}

implementation/endpoints/include/netlink_connector.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,8 @@ class nl_protocol {
111111
nl_protocol() {
112112
proto = 0;
113113
}
114-
nl_protocol(int proto) {
115-
this->proto = proto;
114+
nl_protocol(int _proto) {
115+
proto = _proto;
116116
}
117117
/// Obtain an identifier for the type of the protocol.
118118
int type() const

implementation/endpoints/src/endpoint_manager_impl.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,9 +1109,9 @@ endpoint_manager_impl::find_remote_client(
11091109
service_instances_[_service][its_endpoint.get()] = _instance;
11101110

11111111
// add endpoint to serviceinfo object
1112-
auto found_service_info = rm_->find_service(_service,_instance);
1113-
if (found_service_info) {
1114-
found_service_info->set_endpoint(its_endpoint, _reliable);
1112+
auto found_service_info_inner = rm_->find_service(_service,_instance);
1113+
if (found_service_info_inner) {
1114+
found_service_info_inner->set_endpoint(its_endpoint, _reliable);
11151115
}
11161116
}
11171117
}

implementation/endpoints/src/local_tcp_server_endpoint_impl.cpp

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ bool local_tcp_server_endpoint_impl::is_local() const {
5757

5858
void local_tcp_server_endpoint_impl::init(const endpoint_type& _local,
5959
boost::system::error_code& _error) {
60-
std::lock_guard<std::mutex> its_lock(acceptor_mutex_);
60+
std::scoped_lock its_lock{acceptor_mutex_};
6161
init_unlocked(_local, _error);
6262
}
6363

@@ -99,7 +99,7 @@ void local_tcp_server_endpoint_impl::deinit() {
9999

100100
void local_tcp_server_endpoint_impl::start() {
101101

102-
std::lock_guard<std::mutex> its_lock(acceptor_mutex_);
102+
std::scoped_lock its_lock{acceptor_mutex_};
103103
if (!acceptor_->is_open()) {
104104
boost::system::error_code its_error;
105105
init_unlocked(local_, its_error);
@@ -113,7 +113,7 @@ void local_tcp_server_endpoint_impl::start() {
113113
io_);
114114

115115
{
116-
std::unique_lock<std::mutex> its_lock(new_connection->get_socket_lock());
116+
std::unique_lock its_lock_inner{new_connection->get_socket_lock()};
117117
acceptor_->async_accept(
118118
new_connection->get_socket(),
119119
std::bind(&local_tcp_server_endpoint_impl::accept_cbk,
@@ -128,14 +128,14 @@ void local_tcp_server_endpoint_impl::stop() {
128128

129129
server_endpoint_impl::stop();
130130
{
131-
std::lock_guard<std::mutex> its_lock(acceptor_mutex_);
131+
std::scoped_lock its_lock{acceptor_mutex_};
132132
if (acceptor_->is_open()) {
133133
boost::system::error_code its_error;
134134
acceptor_->close(its_error);
135135
}
136136
}
137137
{
138-
std::lock_guard<std::mutex> its_lock(connections_mutex_);
138+
std::scoped_lock its_lock{connections_mutex_};
139139
for (const auto &c : connections_) {
140140
c.second->stop();
141141
}
@@ -151,7 +151,7 @@ bool local_tcp_server_endpoint_impl::send(const uint8_t *_data, uint32_t _size)
151151
msg << std::hex << std::setfill('0') << std::setw(2) << static_cast<int>(_data[i]) << " ";
152152
VSOMEIP_INFO << msg.str();
153153
#endif
154-
std::lock_guard<std::mutex> its_lock(mutex_);
154+
std::scoped_lock its_lock{mutex_};
155155
if (endpoint_impl::sending_blocked_) {
156156
return false;
157157
}
@@ -161,7 +161,7 @@ bool local_tcp_server_endpoint_impl::send(const uint8_t *_data, uint32_t _size)
161161

162162
connection::ptr its_connection;
163163
{
164-
std::lock_guard<std::mutex> its_lock(connections_mutex_);
164+
std::scoped_lock its_lock_inner{connections_mutex_};
165165
const auto its_iterator = connections_.find(its_client);
166166
if (its_iterator == connections_.end()) {
167167
return false;
@@ -221,7 +221,7 @@ bool local_tcp_server_endpoint_impl::add_connection(const client_t &_client,
221221
const std::shared_ptr<connection> &_connection) {
222222

223223
bool ret = false;
224-
std::lock_guard<std::mutex> its_lock(connections_mutex_);
224+
std::scoped_lock its_lock{connections_mutex_};
225225
auto find_connection = connections_.find(_client);
226226
if (find_connection == connections_.end()) {
227227
connections_[_client] = _connection;
@@ -236,7 +236,7 @@ bool local_tcp_server_endpoint_impl::add_connection(const client_t &_client,
236236
void local_tcp_server_endpoint_impl::remove_connection(
237237
const client_t &_client) {
238238

239-
std::lock_guard<std::mutex> its_lock(connections_mutex_);
239+
std::scoped_lock its_lock{connections_mutex_};
240240
if (!connections_.erase(_client)) {
241241
VSOMEIP_WARNING << "Client " << std::hex << _client << " has no registered connection to "
242242
<< " remove, endpoint > " << this;
@@ -321,8 +321,8 @@ void local_tcp_server_endpoint_impl::accept_cbk(
321321
auto its_ep = std::dynamic_pointer_cast<local_tcp_server_endpoint_impl>(
322322
shared_from_this());
323323
its_timer->async_wait([its_timer, its_ep]
324-
(const boost::system::error_code& _error) {
325-
if (!_error) {
324+
(const boost::system::error_code& _error_inner) {
325+
if (!_error_inner) {
326326
its_ep->start();
327327
}
328328
});
@@ -378,7 +378,7 @@ local_tcp_server_endpoint_impl::connection::get_socket_lock() {
378378
}
379379

380380
void local_tcp_server_endpoint_impl::connection::start() {
381-
std::lock_guard<std::mutex> its_lock(socket_mutex_);
381+
std::scoped_lock its_lock{socket_mutex_};
382382
if (socket_->is_open()) {
383383
const std::size_t its_capacity(recv_buffer_.capacity());
384384
if (recv_buffer_size_ > its_capacity) {
@@ -431,7 +431,7 @@ void local_tcp_server_endpoint_impl::connection::start() {
431431
}
432432

433433
void local_tcp_server_endpoint_impl::connection::stop() {
434-
std::lock_guard<std::mutex> its_lock(socket_mutex_);
434+
std::scoped_lock its_lock{socket_mutex_};
435435
is_stopped_ = true;
436436
if (socket_->is_open()) {
437437
#if defined(__linux__) || defined(ANDROID) || defined(__QNX__)
@@ -474,7 +474,7 @@ void local_tcp_server_endpoint_impl::connection::send_queued(
474474
bufs.push_back(boost::asio::buffer(its_end_tag));
475475

476476
{
477-
std::lock_guard<std::mutex> its_lock(socket_mutex_);
477+
std::scoped_lock its_lock{socket_mutex_};
478478
socket_->async_write(bufs,
479479
std::bind(&local_tcp_server_endpoint_impl::connection::send_cbk,
480480
shared_from_this(), _buffer, std::placeholders::_1,
@@ -617,7 +617,7 @@ void local_tcp_server_endpoint_impl::connection::receive_cbk(
617617
}
618618
if (its_command_size && max_message_size_ != MESSAGE_SIZE_UNLIMITED
619619
&& its_command_size > max_message_size_) {
620-
std::lock_guard<std::mutex> its_lock(socket_mutex_);
620+
std::scoped_lock its_lock{socket_mutex_};
621621
VSOMEIP_ERROR << "Received a local message which exceeds "
622622
<< "maximum message size (" << std::dec << its_command_size
623623
<< ") aborting! local: " << get_path_local() << " remote: "
@@ -903,7 +903,7 @@ local_tcp_server_endpoint_impl::connection::get_recv_buffer_capacity() const {
903903

904904
void
905905
local_tcp_server_endpoint_impl::connection::shutdown_and_close() {
906-
std::lock_guard<std::mutex> its_lock(socket_mutex_);
906+
std::scoped_lock its_lock{socket_mutex_};
907907
shutdown_and_close_unlocked();
908908
}
909909

@@ -915,10 +915,10 @@ local_tcp_server_endpoint_impl::connection::shutdown_and_close_unlocked() {
915915
}
916916

917917
void local_tcp_server_endpoint_impl::print_status() {
918-
std::lock_guard<std::mutex> its_lock(mutex_);
918+
std::scoped_lock its_lock{mutex_};
919919
connections_t its_connections;
920920
{
921-
std::lock_guard<std::mutex> its_lock(connections_mutex_);
921+
std::scoped_lock its_lock_inner{connections_mutex_};
922922
its_connections = connections_;
923923
}
924924
std::string its_local_path("TCP");

0 commit comments

Comments
 (0)