a
/
badguardhome
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Pull request: all: avoid fallthrough 

Merge in DNS/adguard-home from rm-fallthrough to master

Squashed commit of the following:

commit 14308b6cd3580d2c51b6da9f40b8a37766046708
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri May 21 18:16:42 2021 +0300

    dhcpd: imp code

commit 2009219df7d35713d06848010ce57b387e407c0e
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri May 21 17:54:38 2021 +0300

    all: avoid fallthrough
This commit is contained in:
Ainar Garipov 2021-05-21 18:30:57 +03:00
parent 7f2f8de922
commit 6847f7c8bb
5 changed files with 57 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
HACKING.md
View File

@ -69,7 +69,9 @@ on GitHub and most other Markdown renderers. -->
first statement. If all you want there is a log message, use first statement. If all you want there is a log message, use
`agherr.LogPanic`. `agherr.LogPanic`.
* Avoid `errors.New`, use `aghnet.Error` instead. * Avoid `fallthrough`. It makes it harder to rearrange `case`s, to reason
about the code, and also to switch the code to a handler approach, if that
becomes necessary later.
* Avoid `goto`. * Avoid `goto`.

46
internal/dhcpd/v46.go
View File

@ -84,40 +84,42 @@ func ifaceDNSIPAddrs(
backoff time.Duration, backoff time.Duration,
) (addrs []net.IP, err error) { ) (addrs []net.IP, err error) {
var n int var n int
waitForIP:
for n = 1; n <= maxAttempts; n++ { for n = 1; n <= maxAttempts; n++ {
addrs, err = ifaceIPAddrs(iface, ipv) addrs, err = ifaceIPAddrs(iface, ipv)
if err != nil { if err != nil {
return nil, fmt.Errorf("getting ip addrs: %w", err) return nil, fmt.Errorf("getting ip addrs: %w", err)
} }
switch len(addrs) { if len(addrs) > 0 {
case 0: break
log.Debug("dhcpv%d: attempt %d: no ip addresses", ipv, n)
time.Sleep(backoff)
case 1:
// Some Android devices use 8.8.8.8 if there is not
// a secondary DNS server. Fix that by setting the
// secondary DNS address to the same address.
//
// See https://github.com/AdguardTeam/AdGuardHome/issues/1708.
log.Debug("dhcpv%d: setting secondary dns ip to itself", ipv)
addrs = append(addrs, addrs[0])
fallthrough
default:
break waitForIP
} }
log.Debug("dhcpv%d: attempt %d: no ip addresses", ipv, n)
time.Sleep(backoff)
} }
if len(addrs) == 0 { switch len(addrs) {
case 0:
// Don't return errors in case the users want to try and enable // Don't return errors in case the users want to try and enable
// the DHCP server later. // the DHCP server later.
log.Error("dhcpv%d: no ip address for interface after %d attempts and %s", ipv, n, time.Duration(n)*backoff) t := time.Duration(n) * backoff
} else { log.Error("dhcpv%d: no ip for iface after %d attempts and %s", ipv, n, t)
log.Debug("dhcpv%d: got addresses %s after %d attempts", ipv, addrs, n)
return nil, nil
case 1:
// Some Android devices use 8.8.8.8 if there is not a secondary
// DNS server. Fix that by setting the secondary DNS address to
// the same address.
//
// See https://github.com/AdguardTeam/AdGuardHome/issues/1708.
log.Debug("dhcpv%d: setting secondary dns ip to itself", ipv)
addrs = append(addrs, addrs[0])
default:
// Go on.
} }
log.Debug("dhcpv%d: got addresses %s after %d attempts", ipv, addrs, n)
return addrs, nil return addrs, nil
} }

44
internal/dhcpd/v6.go
View File

@ -66,7 +66,8 @@ func (s *v6Server) ResetLeases(ll []*Lease) {
if l.Expiry.Unix() != leaseExpireStatic && if l.Expiry.Unix() != leaseExpireStatic &&
!ip6InRange(s.conf.ipStart, l.IP) { !ip6InRange(s.conf.ipStart, l.IP) {
log.Debug("DHCPv6: skipping a lease with IP %v: not within current IP range", l.IP) log.Debug("dhcpv6: skipping a lease with IP %v: not within current IP range", l.IP)
continue continue
} }
@ -122,7 +123,7 @@ func (s *v6Server) FindMACbyIP(ip net.IP) net.HardwareAddr {
// Remove (swap) lease by index // Remove (swap) lease by index
func (s *v6Server) leaseRemoveSwapByIndex(i int) { func (s *v6Server) leaseRemoveSwapByIndex(i int) {
s.ipAddrs[s.leases[i].IP[15]] = 0 s.ipAddrs[s.leases[i].IP[15]] = 0
log.Debug("DHCPv6: removed lease %s", s.leases[i].HWAddr) log.Debug("dhcpv6: removed lease %s", s.leases[i].HWAddr)
n := len(s.leases) n := len(s.leases)
if i != n-1 { if i != n-1 {
@ -220,7 +221,7 @@ func (s *v6Server) RemoveStaticLease(l Lease) (err error) {
func (s *v6Server) addLease(l *Lease) { func (s *v6Server) addLease(l *Lease) {
s.leases = append(s.leases, l) s.leases = append(s.leases, l)
s.ipAddrs[l.IP[15]] = 1 s.ipAddrs[l.IP[15]] = 1
log.Debug("DHCPv6: added lease %s <-> %s", l.IP, l.HWAddr) log.Debug("dhcpv6: added lease %s <-> %s", l.IP, l.HWAddr)
} }
// Remove a lease with the same properties // Remove a lease with the same properties
@ -321,6 +322,7 @@ func (s *v6Server) checkCID(msg *dhcpv6.Message) error {
if msg.Options.ClientID() == nil { if msg.Options.ClientID() == nil {
return fmt.Errorf("dhcpv6: no ClientID option in request") return fmt.Errorf("dhcpv6: no ClientID option in request")
} }
return nil return nil
} }
@ -336,15 +338,14 @@ func (s *v6Server) checkSID(msg *dhcpv6.Message) error {
if sid != nil { if sid != nil {
return fmt.Errorf("dhcpv6: drop packet: ServerID option in message %s", msg.Type().String()) return fmt.Errorf("dhcpv6: drop packet: ServerID option in message %s", msg.Type().String())
} }
case dhcpv6.MessageTypeRequest, case dhcpv6.MessageTypeRequest,
dhcpv6.MessageTypeRenew, dhcpv6.MessageTypeRenew,
dhcpv6.MessageTypeRelease, dhcpv6.MessageTypeRelease,
dhcpv6.MessageTypeDecline: dhcpv6.MessageTypeDecline:
if sid == nil { if sid == nil {
return fmt.Errorf("dhcpv6: drop packet: no ServerID option in message %s", msg.Type().String()) return fmt.Errorf("dhcpv6: drop packet: no ServerID option in message %s", msg.Type().String())
} }
if !sid.Equal(s.sid) { if !sid.Equal(s.sid) {
return fmt.Errorf("dhcpv6: drop packet: mismatched ServerID option in message %s: %s", return fmt.Errorf("dhcpv6: drop packet: mismatched ServerID option in message %s: %s",
msg.Type().String(), sid.String()) msg.Type().String(), sid.String())
@ -417,13 +418,14 @@ func (s *v6Server) process(msg *dhcpv6.Message, req, resp dhcpv6.DHCPv6) bool {
mac, err := dhcpv6.ExtractMAC(req) mac, err := dhcpv6.ExtractMAC(req)
if err != nil { if err != nil {
log.Debug("DHCPv6: dhcpv6.ExtractMAC: %s", err) log.Debug("dhcpv6: dhcpv6.ExtractMAC: %s", err)
return false return false
} }
lease := s.findLease(mac) lease := s.findLease(mac)
if lease == nil { if lease == nil {
log.Debug("DHCPv6: no lease for: %s", mac) log.Debug("dhcpv6: no lease for: %s", mac)
switch msg.Type() { switch msg.Type() {
@ -440,7 +442,8 @@ func (s *v6Server) process(msg *dhcpv6.Message, req, resp dhcpv6.DHCPv6) bool {
err = s.checkIA(msg, lease) err = s.checkIA(msg, lease)
if err != nil { if err != nil {
log.Debug("DHCPv6: %s", err) log.Debug("dhcpv6: %s", err)
return false return false
} }
@ -497,11 +500,12 @@ func (s *v6Server) process(msg *dhcpv6.Message, req, resp dhcpv6.DHCPv6) bool {
func (s *v6Server) packetHandler(conn net.PacketConn, peer net.Addr, req dhcpv6.DHCPv6) { func (s *v6Server) packetHandler(conn net.PacketConn, peer net.Addr, req dhcpv6.DHCPv6) {
msg, err := req.GetInnerMessage() msg, err := req.GetInnerMessage()
if err != nil { if err != nil {
log.Error("DHCPv6: %s", err) log.Error("dhcpv6: %s", err)
return return
} }
log.Debug("DHCPv6: received: %s", req.Summary()) log.Debug("dhcpv6: received: %s", req.Summary())
err = s.checkCID(msg) err = s.checkCID(msg)
if err != nil { if err != nil {
@ -521,10 +525,11 @@ func (s *v6Server) packetHandler(conn net.PacketConn, peer net.Addr, req dhcpv6.
case dhcpv6.MessageTypeSolicit: case dhcpv6.MessageTypeSolicit:
if msg.GetOneOption(dhcpv6.OptionRapidCommit) == nil { if msg.GetOneOption(dhcpv6.OptionRapidCommit) == nil {
resp, err = dhcpv6.NewAdvertiseFromSolicit(msg) resp, err = dhcpv6.NewAdvertiseFromSolicit(msg)
break break
} }
fallthrough
resp, err = dhcpv6.NewReplyFromMessage(msg)
case dhcpv6.MessageTypeRequest, case dhcpv6.MessageTypeRequest,
dhcpv6.MessageTypeConfirm, dhcpv6.MessageTypeConfirm,
dhcpv6.MessageTypeRenew, dhcpv6.MessageTypeRenew,
@ -532,14 +537,14 @@ func (s *v6Server) packetHandler(conn net.PacketConn, peer net.Addr, req dhcpv6.
dhcpv6.MessageTypeRelease, dhcpv6.MessageTypeRelease,
dhcpv6.MessageTypeInformationRequest: dhcpv6.MessageTypeInformationRequest:
resp, err = dhcpv6.NewReplyFromMessage(msg) resp, err = dhcpv6.NewReplyFromMessage(msg)
default: default:
log.Error("DHCPv6: message type %d not supported", msg.Type()) log.Error("dhcpv6: message type %d not supported", msg.Type())
return return
} }
if err != nil { if err != nil {
log.Error("DHCPv6: %s", err) log.Error("dhcpv6: %s", err)
return return
} }
@ -547,11 +552,12 @@ func (s *v6Server) packetHandler(conn net.PacketConn, peer net.Addr, req dhcpv6.
_ = s.process(msg, req, resp) _ = s.process(msg, req, resp)
log.Debug("DHCPv6: sending: %s", resp.Summary()) log.Debug("dhcpv6: sending: %s", resp.Summary())
_, err = conn.WriteTo(resp.ToBytes(), peer) _, err = conn.WriteTo(resp.ToBytes(), peer)
if err != nil { if err != nil {
log.Error("DHCPv6: conn.Write to %s failed: %s", peer, err) log.Error("dhcpv6: conn.Write to %s failed: %s", peer, err)
return return
} }
} }
@ -660,10 +666,10 @@ func (s *v6Server) Stop() {
return return
} }
log.Debug("DHCPv6: stopping") log.Debug("dhcpv6: stopping")
err = s.srv.Close() err = s.srv.Close()
if err != nil { if err != nil {
log.Error("DHCPv6: srv.Close: %s", err) log.Error("dhcpv6: srv.Close: %s", err)
} }
// now server.Serve() will return // now server.Serve() will return

8
internal/dnsforward/stats.go
View File

@ -92,11 +92,9 @@ func (s *Server) updateStats(ctx *dnsContext, elapsed time.Duration, res filteri
e.Result = stats.RParental e.Result = stats.RParental
case filtering.FilteredSafeSearch: case filtering.FilteredSafeSearch:
e.Result = stats.RSafeSearch e.Result = stats.RSafeSearch
case filtering.FilteredBlockList: case filtering.FilteredBlockList,
fallthrough filtering.FilteredInvalid,
case filtering.FilteredInvalid: filtering.FilteredBlockedService:
fallthrough
case filtering.FilteredBlockedService:
e.Result = stats.RFiltered e.Result = stats.RFiltered
} }

2
scripts/make/go-lint.sh
View File

@ -106,6 +106,8 @@ underscores() {
} }
} }
# TODO(a.garipov): Add an analyser to look for `fallthrough`, `goto`, and `new`?
# Helpers # Helpers