Skip to content

Commit cdf84a5

Browse files
vanvughtvanvught
committed
Refactor: improve code quality in lib-network
- Rename short/ambiguous parameter names (gw→gateway, fn→ext_callback_fn, l→listener, r→rtx, q→queue, d→port_data, c→tcb, etc.) for clarity - Improve bitflag checks to use explicit equality comparisons - Fix const-correctness (const auto* src, const auto* ptr) - Use 1U instead of 1u for unsigned literal consistency - Reformat kServices array initializer and fix else-after-return patterns - Remove redundant null checks before delete[] - Remove redundant return at end of void function in udp.cpp - Change Free() to void return type usage in network_memory.h
1 parent b84abd8 commit cdf84a5

9 files changed

Lines changed: 136 additions & 138 deletions

File tree

‎lib-network/include/core/netif.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,15 @@ inline uint32_t Netmask() {
118118
return netif::global::netif_default.netmask.addr;
119119
}
120120

121-
void SetGw(network::ip4_addr_t gw);
121+
void SetGw(network::ip4_addr_t gateway);
122122

123123
inline uint32_t Gw() {
124124
return netif::global::netif_default.gw.addr;
125125
}
126126

127-
void SetAddr(network::ip4_addr_t ipaddr, network::ip4_addr_t netmask, network::ip4_addr_t gw);
127+
void SetAddr(network::ip4_addr_t ipaddr, network::ip4_addr_t netmask, network::ip4_addr_t gateway);
128128

129-
void AddExtCallback(netif_ext_callback_fn fn);
129+
void AddExtCallback(netif_ext_callback_fn ext_callback_fn);
130130

131131
inline uint32_t BroadcastIpAddr() {
132132
return netif::global::netif_default.broadcast_ip.addr;

‎lib-network/include/network_tcp.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ using CallbackConnect = void (*)(ConnHandle, Event, void*);
5050
ConnHandle Connect(uint32_t remote_ip, uint16_t remote_port, CallbackConnect cb_connect, CallbackData cb_data, void* context);
5151

5252
// Common
53-
int32_t Send(ConnHandle connection_handle, const uint8_t* buffer, uint32_t length);
54-
int32_t Close(ConnHandle connection_handle); // graceful FIN
55-
void Abort(ConnHandle connection_handle); // RST
53+
int32_t Send(ConnHandle conn_handle, const uint8_t* buffer, uint32_t length);
54+
int32_t Close(ConnHandle conn_handle); // graceful FIN
55+
void Abort(ConnHandle conn_handle); // RST
5656
} // namespace network::tcp
5757

5858
#endif // NETWORK_TCP_H_

‎lib-network/src/apps/mdns/mdns.cpp‎

Lines changed: 23 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -92,17 +92,16 @@ struct Service {
9292
const uint16_t kPortDefault;
9393
};
9494

95-
static constexpr Service kServices[]{
96-
{kDomainConfig, sizeof(kDomainConfig), Protocols::kUdp, 0x2905},
97-
{kDomainTftp, sizeof(kDomainTftp), Protocols::kUdp,network::iana::Ports::kPortTftp},
98-
{kDomainHttp, sizeof(kDomainHttp), Protocols::kTcp, network::iana::Ports::kPortHttp},
99-
{kDomainHttp, sizeof(kDomainHttp), Protocols::kTcp, network::iana::Ports::kPortHttpAlt},
100-
{kDomainRdmnetLlrp, sizeof(kDomainRdmnetLlrp), Protocols::kUdp, 5569},
101-
{kDomainNtp, sizeof(kDomainNtp), Protocols::kUdp, network::iana::Ports::kPortNtp},
102-
{kDomainMidi, sizeof(kDomainMidi), Protocols::kUdp, 5004},
103-
{kDomainOsc, sizeof(kDomainOsc), Protocols::kUdp, 0},
104-
{kDomainDdp, sizeof(kDomainDdp), Protocols::kUdp, 4048},
105-
{kDomainPp, sizeof(kDomainPp), Protocols::kUdp, 5078}};
95+
static constexpr Service kServices[]{{kDomainConfig, sizeof(kDomainConfig), Protocols::kUdp, 0x2905},
96+
{kDomainTftp, sizeof(kDomainTftp), Protocols::kUdp, network::iana::Ports::kPortTftp},
97+
{kDomainHttp, sizeof(kDomainHttp), Protocols::kTcp, network::iana::Ports::kPortHttp},
98+
{kDomainHttp, sizeof(kDomainHttp), Protocols::kTcp, network::iana::Ports::kPortHttpAlt},
99+
{kDomainRdmnetLlrp, sizeof(kDomainRdmnetLlrp), Protocols::kUdp, 5569},
100+
{kDomainNtp, sizeof(kDomainNtp), Protocols::kUdp, network::iana::Ports::kPortNtp},
101+
{kDomainMidi, sizeof(kDomainMidi), Protocols::kUdp, 5004},
102+
{kDomainOsc, sizeof(kDomainOsc), Protocols::kUdp, 0},
103+
{kDomainDdp, sizeof(kDomainDdp), Protocols::kUdp, 4048},
104+
{kDomainPp, sizeof(kDomainPp), Protocols::kUdp, 5078}};
106105

107106
struct Domain {
108107
uint8_t a_name[kDomainMaxlen];
@@ -354,12 +353,13 @@ static uint8_t* PutDomainNameAsLabels(uint8_t* ptr, Domain const& domain) {
354353
*ptr++ = static_cast<uint8_t>(0xC0 | (offset >> 8));
355354
*ptr++ = static_cast<uint8_t>(offset);
356355
return ptr;
357-
} else {
358-
auto len = *np++;
359-
*ptr++ = len;
360-
for (uint32_t i = 0; i < len; i++) {
361-
*ptr++ = *np++;
362-
}
356+
}
357+
358+
auto len = *np++;
359+
*ptr++ = len;
360+
361+
for (uint32_t i = 0; i < len; i++) {
362+
*ptr++ = *np++;
363363
}
364364
}
365365

@@ -590,11 +590,11 @@ static const uint8_t* GetDomainName(const uint8_t* const kMsg, const uint8_t* pt
590590
}
591591
}
592592

593-
if (nextbyte) {
594-
return (nextbyte);
595-
} else {
596-
return (ptr);
593+
if (nextbyte != nullptr) {
594+
return nextbyte;
597595
}
596+
597+
return (ptr);
598598
}
599599

600600
void Start() {
@@ -618,13 +618,8 @@ void Stop() {
618618
mdns::SendAnnouncement(0);
619619

620620
for (auto& record : s_service_records) {
621-
if (record.name != nullptr) {
622-
delete[] record.name;
623-
}
624-
625-
if (record.text_content != nullptr) {
626-
delete[] record.text_content;
627-
}
621+
delete[] record.name;
622+
delete[] record.text_content;
628623
}
629624

630625
network::igmp::LeaveGroup(s_handle, network::dns::kMulticastAddress);

‎lib-network/src/core/netif.cpp‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ static void NetifDoIpAddrChanged([[maybe_unused]] network::ip4_addr_t old_addr,
8080
static void NetifIssueReports() {
8181
const auto& netif = netif::global::netif_default;
8282

83-
if (!(netif.flags & Netif::kNetifFlagLinkUp)) {
83+
if (!((netif.flags & Netif::kNetifFlagLinkUp) == Netif::kNetifFlagLinkUp)) {
8484
return;
8585
}
8686

@@ -134,14 +134,14 @@ static bool NetifDoSetNetmask(network::ip4_addr_t netmask, network::ip4_addr_t&
134134
return false; // netmask unchanged
135135
}
136136

137-
static bool NetifDoSetGw(network::ip4_addr_t gw, network::ip4_addr_t& old_gw) {
137+
static bool NetifDoSetGw(network::ip4_addr_t gateway, network::ip4_addr_t& old_gw) {
138138
DEBUG_ENTRY();
139139

140140
auto& netif = netif::global::netif_default;
141141

142-
if (gw.addr != netif.gw.addr) {
142+
if (gateway.addr != netif.gw.addr) {
143143
old_gw.addr = netif.gw.addr;
144-
netif.gw.addr = gw.addr;
144+
netif.gw.addr = gateway.addr;
145145

146146
DEBUG_EXIT();
147147
return true; // gateway changed
@@ -171,17 +171,17 @@ void SetNetmask(network::ip4_addr_t netmask) {
171171
}
172172
}
173173

174-
void SetGw(network::ip4_addr_t gw) {
174+
void SetGw(network::ip4_addr_t gateway) {
175175
network::ip4_addr_t old_gw;
176176

177-
if (NetifDoSetGw(gw, old_gw)) {
177+
if (NetifDoSetGw(gateway, old_gw)) {
178178
netif_ext_callback_args_t args;
179179
args.ipv4_changed.old_gw = old_gw;
180180
callback_fn(NetifReason::kIpv4GatewayChanged, &args);
181181
}
182182
}
183183

184-
void SetAddr(network::ip4_addr_t ipaddr, network::ip4_addr_t netmask, network::ip4_addr_t gw) {
184+
void SetAddr(network::ip4_addr_t ipaddr, network::ip4_addr_t netmask, network::ip4_addr_t gateway) {
185185
DEBUG_ENTRY();
186186
DEBUG_PRINTF(IPSTR " " IPSTR " " IPSTR, IP2STR(ipaddr.addr), IP2STR(netmask.addr), IP2STR(gw.addr));
187187

@@ -208,7 +208,7 @@ void SetAddr(network::ip4_addr_t ipaddr, network::ip4_addr_t netmask, network::i
208208
cb_args.ipv4_changed.old_netmask.addr = old_nm.addr;
209209
}
210210

211-
if (NetifDoSetGw(gw, old_gw)) {
211+
if (NetifDoSetGw(gateway, old_gw)) {
212212
change_reason |= NetifReason::kIpv4GatewayChanged;
213213
cb_args.ipv4_changed.old_gw = old_gw;
214214
}
@@ -239,8 +239,8 @@ void SetAddr(network::ip4_addr_t ipaddr, network::ip4_addr_t netmask, network::i
239239
DEBUG_EXIT();
240240
}
241241

242-
void AddExtCallback(netif_ext_callback_fn fn) {
243-
callback_fn = fn;
242+
void AddExtCallback(netif_ext_callback_fn ext_callback_fn) {
243+
callback_fn = ext_callback_fn;
244244
}
245245

246246
// Link
@@ -249,7 +249,7 @@ void SetLinkUp() {
249249
DEBUG_ENTRY();
250250
const auto& netif = netif::global::netif_default;
251251

252-
if (!(netif.flags & Netif::kNetifFlagLinkUp)) {
252+
if (!((netif.flags & Netif::kNetifFlagLinkUp) == Netif::kNetifFlagLinkUp)) {
253253
netif::SetFlags(Netif::kNetifFlagLinkUp);
254254

255255
network::dhcp::NetworkChangedLinkUp();
@@ -273,7 +273,7 @@ void SetLinkDown() {
273273

274274
const auto& netif = netif::global::netif_default;
275275

276-
if (netif.flags & Netif::kNetifFlagLinkUp) {
276+
if ((netif.flags & Netif::kNetifFlagLinkUp) == Netif::kNetifFlagLinkUp) {
277277
netif::ClearFlags(Netif::kNetifFlagLinkUp);
278278

279279
network::autoip::NetworkChangedLinkDown();

‎lib-network/src/core/network_memcpy.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,10 @@ inline uint32_t MemcpyIp(const uint8_t* ip) {
130130

131131
_pcast32 cast;
132132
#ifdef __ARM_ARCH_7A__
133-
volatile auto* src = ip;
133+
volatile const auto* src = ip;
134134
volatile auto* dst = cast.u8;
135135
#else
136-
auto* src = ip;
136+
const auto* src = ip;
137137
auto* dst = cast.u8;
138138
#endif
139139
for (size_t i = 0; i < network::ip4::kAddressLength; i++) {

‎lib-network/src/core/network_memory.h‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class Allocator {
8989
}
9090

9191
const uint32_t kIndex = static_cast<uint32_t>(__builtin_ctz(free_mask_));
92-
free_mask_ &= ~(1u << kIndex);
92+
free_mask_ &= ~(1U << kIndex);
9393

9494
Status();
9595

@@ -107,7 +107,7 @@ class Allocator {
107107
}
108108

109109
const uint32_t kIndex = static_cast<uint32_t>(__builtin_ctz(free_mask_));
110-
free_mask_ &= ~(1u << kIndex);
110+
free_mask_ &= ~(1U << kIndex);
111111

112112
size_[kIndex] = size;
113113
memcpy(pool[kIndex], data, size);
@@ -122,15 +122,18 @@ class Allocator {
122122

123123
for (uint32_t index = 0; index < kBlocks; ++index) {
124124
if (static_cast<void*>(pool[index]) == pointer) {
125-
return Free(static_cast<uint16_t>(index));
125+
Free(static_cast<uint16_t>(index));
126+
return;
126127
}
127128
}
128129

129-
assert(false && "Pointer is not from pool");
130+
assert(false && "Pointer is not from pool");
130131
}
131132

132133
void Free(uint16_t index) {
133-
if (index == UINT16_MAX) return;
134+
if (index == UINT16_MAX) {
135+
return;
136+
}
134137

135138
assert(index < kBlocks);
136139

‎lib-network/src/core/network_private.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@
4141

4242
namespace emac::eth {
4343
uint8_t* SendGetDmaBuffer();
44-
void Send(const uint32_t);
45-
void Send(void*, const uint32_t);
44+
void Send(uint32_t);
45+
void Send(void*, uint32_t);
4646
#if defined CONFIG_NET_ENABLE_PTP
4747
void SendTimestamp(uint32_t);
4848
void SendTimestamp(void*, uint32_t);
@@ -52,19 +52,19 @@ void FreePkt();
5252
} // namespace emac::eth
5353

5454
namespace network {
55-
inline void Error(const char* func, const char* s) {
56-
printf("%s: %s\n", func, s);
55+
inline void Error(const char* func, const char* string) {
56+
printf("%s: %s\n", func, string);
5757
}
5858

59-
#define ERROR(s) Error(__func__, (s))
59+
#define ERROR(s) Error(__func__, (s))
6060

6161
namespace global {
6262
extern uint32_t broadcast_mask;
6363
extern uint32_t on_network_mask;
6464
} // namespace global
6565

6666
inline uint16_t Chksum(const void* data, uint32_t length) {
67-
auto* ptr = reinterpret_cast<const uint16_t*>(data);
67+
const auto* ptr = reinterpret_cast<const uint16_t*>(data);
6868
uint32_t sum = 0;
6969

7070
while (length > 1) {
@@ -79,7 +79,7 @@ inline uint16_t Chksum(const void* data, uint32_t length) {
7979
}
8080

8181
// Fold 32-bit sum into 16 bits
82-
while (sum >> 16) {
82+
while ((sum >> 16) != 0) {
8383
sum = (sum >> 16) + (sum & 0xFFFF);
8484
}
8585

0 commit comments

Comments
 (0)