net: Minor accumulated cleanups #12855

pull tjps wants to merge 1 commits into bitcoin:master from tjps:tjps_misc_cleans changing 6 files +36 −68

tjps commented at 4:19 AM on April 2, 2018: contributor

From now-derelict larger changes I had been working on, here are a series of DRY refactors/cleanups. Net loss of 35 lines of code - a small step in the good fight.

In particular I think operator!= should only ever be implemented as a negation of operator==. Lower chance for errors, and removes the possibility of divergent behavior.
fanquake added the label P2P on Apr 2, 2018
fanquake commented at 4:20 AM on April 2, 2018: member

Looks like this needs a rebase.
fanquake requested review from theuni on Apr 5, 2018
fanquake removed review request from theuni on Apr 5, 2018
fanquake requested review from sipa on Apr 5, 2018
fanquake requested review from theuni on Apr 5, 2018
in src/netaddress.cpp:462 in 27213212dd outdated
```
 458 | @@ -469,14 +459,8 @@ int CNetAddr::GetReachabilityFrom(const CNetAddr *paddrPartner) const
 459 |      }
 460 |  }
 461 |  
 462 | -void CService::Init()
 463 | +CService::CService() : port(0)
```
laanwj commented at 10:17 AM on April 5, 2018:

As you change this initializer to a different style anyway: maybe do the initialization in-line where the field is defined?

theuni commented at 6:23 PM on April 9, 2018:

+1
laanwj approved
laanwj commented at 10:18 AM on April 5, 2018: member

LGTM one small suggestion
practicalswift commented at 9:30 PM on April 5, 2018: contributor

Nice cleanups!

Concept ACK
in src/netaddress.h:91 in 27213212dd outdated
```
  86 | @@ -86,7 +87,7 @@ class CNetAddr
  87 |          bool GetIn6Addr(struct in6_addr* pipv6Addr) const;
  88 |  
  89 |          friend bool operator==(const CNetAddr& a, const CNetAddr& b);
  90 | -        friend bool operator!=(const CNetAddr& a, const CNetAddr& b);
  91 | +        friend bool operator!=(const CNetAddr& a, const CNetAddr& b) { return !(a == b); }
```
theuni commented at 6:22 PM on April 9, 2018:

Just because negation is that obvious? I suspect this is some common coding practice?

tjps commented at 8:21 PM on April 16, 2018:

In several different projects I have worked on this is a standard practice. It is simpler, less error-prone (in the case where someone updates operator== but forgets to update operator!=) and usually is also a LoC reduction.

This could be applied in other areas of the codebase as well but I didn't want to scope creep this PR.
theuni commented at 6:27 PM on April 9, 2018: member

Looks good to me too other than what @laanwj pointed out.
in src/compat.h:100 in 27213212dd outdated
```
  95 | @@ -96,6 +96,12 @@ typedef int32_t ssize_t;
  96 |  size_t strnlen( const char *start, size_t max_len);
  97 |  #endif // HAVE_DECL_STRNLEN
  98 |  
  99 | +#ifndef WIN32
 100 | +typedef void* sockopt_arg_t;
```
sipa commented at 4:42 PM on April 10, 2018:

Type names ending in _t are technically reserved by POSIX, so risk colliding with system types on some architectures.

tjps commented at 8:20 PM on April 16, 2018:

Good point, I totally forgot that _t was already covered by a naming convention. Will update.
sipa commented at 4:44 PM on April 10, 2018: member

utACK 27213212dde0ae5cb7abc10c347c663b34aa0469, one nit.
laanwj commented at 2:55 PM on April 16, 2018: member

Looks like this is ready for merge apart from @sipa's nit...
net: Minor accumulated cleanups 2c084a6609
laanwj merged this on Apr 19, 2018
laanwj closed this on Apr 19, 2018
laanwj referenced this in commit c199869408 on Apr 19, 2018
tjps deleted the branch on May 13, 2018
PastaPastaPasta referenced this in commit 8d0def0fa3 on Jan 26, 2020
PastaPastaPasta referenced this in commit c7ed2d2498 on Jan 26, 2020
PastaPastaPasta referenced this in commit 60a0aad0a6 on Jan 27, 2020
PastaPastaPasta referenced this in commit e7beff5bcd on Jan 27, 2020
ckti referenced this in commit 114a3e28e9 on Mar 28, 2021
gades referenced this in commit db431ee872 on Jun 25, 2021
random-zebra referenced this in commit b4751e10ce on Aug 11, 2021
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#33378 Remove unnecessary casts when calling socket operations