Message ID | 87k2hhmp8q.fsf@frog.home |
---|---|
State | Accepted, archived |
Delegated to: | stephen hemminger |
Headers | show |
On Wed, Jun 22, 2016 at 01:34:13PM +0200, Jakub Sitnicki wrote: > On Tue, Jun 21, 2016 at 06:18 PM CEST, Phil Sutter <phil@nwl.cc> wrote: [...] > > - a = attrs[TCP_METRICS_ATTR_ADDR_IPV4]; > > - if (a) { > > + if ((a = attrs[TCP_METRICS_ATTR_ADDR_IPV4])) { > > Copy the pointer inside the branch? > > Same gain on indentation while keeping checkpatch happy. Good point! Thanks for the patch. :) Cheers, Phil
From: Jakub Sitnicki > Sent: 22 June 2016 12:34 ... > > - a = attrs[TCP_METRICS_ATTR_ADDR_IPV4]; > > - if (a) { > > + if ((a = attrs[TCP_METRICS_ATTR_ADDR_IPV4])) { > > Copy the pointer inside the branch? > > Same gain on indentation while keeping checkpatch happy. Or as below (hacked from the mails): a = attrs[TCP_METRICS_ATTR_ADDR_IPV4]; if (a) { if (f.daddr.family && f.daddr.family != AF_INET) return 0; memcpy(&daddr.data, RTA_DATA(a), 4); daddr.bytelen = 4; family = AF_INET; atype = TCP_METRICS_ATTR_ADDR_IPV4; dlen = RTA_PAYLOAD(a); } else { a = attrs[TCP_METRICS_ATTR_ADDR_IPV6]; if (!a) return 0; memcpy(&daddr.data, RTA_DATA(a), 16); daddr.bytelen = 16; family = AF_INET6; atype = TCP_METRICS_ATTR_ADDR_IPV6; dlen = RTA_PAYLOAD(a); } David
Hi David, On Wed, Jun 22, 2016 at 01:00:08PM +0000, David Laight wrote: > From: Jakub Sitnicki > > Sent: 22 June 2016 12:34 > ... > > > - a = attrs[TCP_METRICS_ATTR_ADDR_IPV4]; > > > - if (a) { > > > + if ((a = attrs[TCP_METRICS_ATTR_ADDR_IPV4])) { > > > > Copy the pointer inside the branch? > > > > Same gain on indentation while keeping checkpatch happy. > > Or as below (hacked from the mails): > > a = attrs[TCP_METRICS_ATTR_ADDR_IPV4]; > if (a) { > if (f.daddr.family && f.daddr.family != AF_INET) > return 0; > memcpy(&daddr.data, RTA_DATA(a), 4); > daddr.bytelen = 4; > family = AF_INET; > atype = TCP_METRICS_ATTR_ADDR_IPV4; > dlen = RTA_PAYLOAD(a); > } else { > a = attrs[TCP_METRICS_ATTR_ADDR_IPV6]; > if (!a) > return 0; > memcpy(&daddr.data, RTA_DATA(a), 16); > daddr.bytelen = 16; > family = AF_INET6; > atype = TCP_METRICS_ATTR_ADDR_IPV6; > dlen = RTA_PAYLOAD(a); > } Yes, this should work as well and require less code changes overall. Though I think it's less readable: | if (a) { | do a; | else { | if (!b) | return; | do b; | } vs. | if (a) { | do a; | } else if (b) { | do b; | } else { | return; | } Don't you think? Thanks, Phil
diff --git a/ip/tcp_metrics.c b/ip/tcp_metrics.c index f82604f..ac2613a 100644 --- a/ip/tcp_metrics.c +++ b/ip/tcp_metrics.c @@ -112,47 +112,44 @@ static int process_msg(const struct sockaddr_nl *who, struct nlmsghdr *n, parse_rtattr(attrs, TCP_METRICS_ATTR_MAX, (void *) ghdr + GENL_HDRLEN, len); - a = attrs[TCP_METRICS_ATTR_ADDR_IPV4]; - if (a) { + if (attrs[TCP_METRICS_ATTR_ADDR_IPV4]) { if (f.daddr.family && f.daddr.family != AF_INET) return 0; + a = attrs[TCP_METRICS_ATTR_ADDR_IPV4]; memcpy(&daddr.data, RTA_DATA(a), 4); daddr.bytelen = 4; family = AF_INET; atype = TCP_METRICS_ATTR_ADDR_IPV4; dlen = RTA_PAYLOAD(a); - } else { - a = attrs[TCP_METRICS_ATTR_ADDR_IPV6]; - if (a) { - if (f.daddr.family && f.daddr.family != AF_INET6) - return 0; - memcpy(&daddr.data, RTA_DATA(a), 16); - daddr.bytelen = 16; - family = AF_INET6; - atype = TCP_METRICS_ATTR_ADDR_IPV6; - dlen = RTA_PAYLOAD(a); - } else + } else if (attrs[TCP_METRICS_ATTR_ADDR_IPV6]) { + if (f.daddr.family && f.daddr.family != AF_INET6) return 0; + a = attrs[TCP_METRICS_ATTR_ADDR_IPV6]; + memcpy(&daddr.data, RTA_DATA(a), 16); + daddr.bytelen = 16; + family = AF_INET6; + atype = TCP_METRICS_ATTR_ADDR_IPV6; + dlen = RTA_PAYLOAD(a); + } else { + return 0; } - a = attrs[TCP_METRICS_ATTR_SADDR_IPV4]; - if (a) { + if (attrs[TCP_METRICS_ATTR_SADDR_IPV4]) { if (f.saddr.family && f.saddr.family != AF_INET) return 0; + a = attrs[TCP_METRICS_ATTR_SADDR_IPV4]; memcpy(&saddr.data, RTA_DATA(a), 4); saddr.bytelen = 4; stype = TCP_METRICS_ATTR_SADDR_IPV4; slen = RTA_PAYLOAD(a); - } else { + } else if (attrs[TCP_METRICS_ATTR_SADDR_IPV6]) { + if (f.saddr.family && f.saddr.family != AF_INET6) + return 0; a = attrs[TCP_METRICS_ATTR_SADDR_IPV6]; - if (a) { - if (f.saddr.family && f.saddr.family != AF_INET6) - return 0; - memcpy(&saddr.data, RTA_DATA(a), 16); - saddr.bytelen = 16; - stype = TCP_METRICS_ATTR_SADDR_IPV6; - slen = RTA_PAYLOAD(a); - } + memcpy(&saddr.data, RTA_DATA(a), 16); + saddr.bytelen = 16; + stype = TCP_METRICS_ATTR_SADDR_IPV6; + slen = RTA_PAYLOAD(a); } if (f.daddr.family && f.daddr.bitlen >= 0 &&