Pull request: all: fix lint and naming issues vol. 2

Merge in DNS/adguard-home from 2276-fix-lint-2 to master

Updates #2276.

Squashed commit of the following:

commit 24760b9586bb31be134ef9518dbece485560b1a0
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Dec 7 14:39:50 2020 +0300

    all: fix lint and naming issues vol. 2

internal/dhcpd/dhcpd.go

@@ -52,6 +52,7 @@ type ServerConfig struct {
 	HTTPRegister func(string, string, func(http.ResponseWriter, *http.Request)) `yaml:"-"`
 }
 
+// OnLeaseChangedT is a callback for lease changes.
 type OnLeaseChangedT func(flags int)
 
 // flags for onLeaseChanged()
@@ -74,6 +75,7 @@ type Server struct {
 	onLeaseChanged []OnLeaseChangedT
 }
 
+// ServerInterface is an interface for servers.
 type ServerInterface interface {
 	Leases(flags int) []Lease
 	SetOnLeaseChanged(onLeaseChanged OnLeaseChangedT)

internal/dhcpd/nclient4/client.go

@@ -19,9 +19,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"log"
 	"net"
-	"os"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -317,26 +315,6 @@ func WithTimeout(d time.Duration) ClientOpt {
 	}
 }
 
-// WithSummaryLogger logs one-line DHCPv4 message summaries when sent & received.
-func WithSummaryLogger() ClientOpt {
-	return func(c *Client) (err error) {
-		c.logger = ShortSummaryLogger{
-			Printfer: log.New(os.Stderr, "[dhcpv4] ", log.LstdFlags),
-		}
-		return
-	}
-}
-
-// WithDebugLogger logs multi-line full DHCPv4 messages when sent & received.
-func WithDebugLogger() ClientOpt {
-	return func(c *Client) (err error) {
-		c.logger = DebugLogger{
-			Printfer: log.New(os.Stderr, "[dhcpv4] ", log.LstdFlags),
-		}
-		return
-	}
-}
-
 // WithLogger set the logger (see interface Logger).
 func WithLogger(newLogger Logger) ClientOpt {
 	return func(c *Client) (err error) {

internal/dhcpd/routeradv.go

@@ -180,15 +180,18 @@ func (ra *raCtx) Init() error {
 	data := createICMPv6RAPacket(params)
 
 	var err error
+	success := false
 	ipAndScope := ra.ipAddr.String() + "%" + ra.ifaceName
 	ra.conn, err = icmp.ListenPacket("ip6:ipv6-icmp", ipAndScope)
 	if err != nil {
 		return fmt.Errorf("dhcpv6 ra: icmp.ListenPacket: %w", err)
 	}
-	success := false
 	defer func() {
 		if !success {
-			ra.Close()
+			cerr := ra.Close()
+			if cerr != nil {
+				log.Error("closing context: %s", cerr)
+			}
 		}
 	}()
 
@@ -227,13 +230,15 @@ func (ra *raCtx) Init() error {
 	return nil
 }
 
-// Close - close module
+// Close closes the module.
-func (ra *raCtx) Close() {
+func (ra *raCtx) Close() (err error) {
 	log.Debug("dhcpv6 ra: closing")
 
 	ra.stop.Store(1)
 
 	if ra.conn != nil {
-		ra.conn.Close()
+		return ra.conn.Close()
 	}
+
+	return nil
 }

internal/dhcpd/v46_windows.go

@@ -1,11 +1,12 @@
+// +build windows
+
 package dhcpd
 
 // 'u-root/u-root' package, a dependency of 'insomniacslk/dhcp' package, doesn't build on Windows
 
 import "net"
 
-type winServer struct {
+type winServer struct{}
-}
 
 func (s *winServer) ResetLeases(leases []*Lease)            {}
 func (s *winServer) GetLeases(flags int) []Lease            { return nil }

internal/dhcpd/v6.go

@@ -628,7 +628,10 @@ func (s *v6Server) Start() error {
 
 // Stop - stop server
 func (s *v6Server) Stop() {
-	s.ra.Close()
+	err := s.ra.Close()
+	if err != nil {
+		log.Error("dhcpv6: s.ra.Close: %s", err)
+	}
 
 	// DHCPv6 server may not be initialized if ra_slaac_only=true
 	if s.srv == nil {
@@ -636,10 +639,11 @@ func (s *v6Server) Stop() {
 	}
 
 	log.Debug("DHCPv6: stopping")
-	err := s.srv.Close()
+	err = s.srv.Close()
 	if err != nil {
 		log.Error("DHCPv6: srv.Close: %s", err)
 	}
+
 	// now server.Serve() will return
 	s.srv = nil
 }

internal/dnsfilter/dnsfilter.go

@@ -263,11 +263,20 @@ func (d *Dnsfilter) Close() {
 }
 
 func (d *Dnsfilter) reset() {
+	var err error
+
 	if d.rulesStorage != nil {
-		_ = d.rulesStorage.Close()
+		err = d.rulesStorage.Close()
+		if err != nil {
+			log.Error("dnsfilter: rulesStorage.Close: %s", err)
+		}
 	}
+
 	if d.rulesStorageWhite != nil {
-		d.rulesStorageWhite.Close()
+		err = d.rulesStorageWhite.Close()
+		if err != nil {
+			log.Error("dnsfilter: rulesStorageWhite.Close: %s", err)
+		}
 	}
 }
 
@@ -336,9 +345,9 @@ func (d *Dnsfilter) CheckHost(host string, qtype uint16, setts *RequestFiltering
 	// Now check the hosts file -- do we have any rules for it?
 	// just like DNS rewrites, it has higher priority than filtering rules.
 	if d.Config.AutoHosts != nil {
-		matched, err := d.checkAutoHosts(host, qtype, &result)
+		matched := d.checkAutoHosts(host, qtype, &result)
 		if matched {
-			return result, err
+			return result, nil
 		}
 	}
 
@@ -403,13 +412,13 @@ func (d *Dnsfilter) CheckHost(host string, qtype uint16, setts *RequestFiltering
 	return Result{}, nil
 }
 
-func (d *Dnsfilter) checkAutoHosts(host string, qtype uint16, result *Result) (matched bool, err error) {
+func (d *Dnsfilter) checkAutoHosts(host string, qtype uint16, result *Result) (matched bool) {
 	ips := d.Config.AutoHosts.Process(host, qtype)
 	if ips != nil {
 		result.Reason = RewriteEtcHosts
 		result.IPList = ips
 
-		return true, nil
+		return true
 	}
 
 	revHosts := d.Config.AutoHosts.ProcessReverse(host, qtype)
@@ -422,10 +431,10 @@ func (d *Dnsfilter) checkAutoHosts(host string, qtype uint16, result *Result) (m
 			result.ReverseHosts[i] = revHosts[i] + "."
 		}
 
-		return true, nil
+		return true
 	}
 
-	return false, nil
+	return false
 }
 
 // Process rewrites table

internal/dnsfilter/safe_search.go

@@ -1,148 +0,0 @@
-package dnsfilter
-
-import (
-	"bytes"
-	"encoding/binary"
-	"encoding/gob"
-	"encoding/json"
-	"fmt"
-	"net"
-	"net/http"
-	"time"
-
-	"github.com/AdguardTeam/golibs/cache"
-	"github.com/AdguardTeam/golibs/log"
-)
-
-/*
-expire byte[4]
-res Result
-*/
-func (d *Dnsfilter) setCacheResult(cache cache.Cache, host string, res Result) int {
-	var buf bytes.Buffer
-
-	expire := uint(time.Now().Unix()) + d.Config.CacheTime*60
-	exp := make([]byte, 4)
-	binary.BigEndian.PutUint32(exp, uint32(expire))
-	_, _ = buf.Write(exp)
-
-	enc := gob.NewEncoder(&buf)
-	err := enc.Encode(res)
-	if err != nil {
-		log.Error("gob.Encode(): %s", err)
-		return 0
-	}
-	val := buf.Bytes()
-	_ = cache.Set([]byte(host), val)
-	return len(val)
-}
-
-func getCachedResult(cache cache.Cache, host string) (Result, bool) {
-	data := cache.Get([]byte(host))
-	if data == nil {
-		return Result{}, false
-	}
-
-	exp := int(binary.BigEndian.Uint32(data[:4]))
-	if exp <= int(time.Now().Unix()) {
-		cache.Del([]byte(host))
-		return Result{}, false
-	}
-
-	var buf bytes.Buffer
-	buf.Write(data[4:])
-	dec := gob.NewDecoder(&buf)
-	r := Result{}
-	err := dec.Decode(&r)
-	if err != nil {
-		log.Debug("gob.Decode(): %s", err)
-		return Result{}, false
-	}
-
-	return r, true
-}
-
-// SafeSearchDomain returns replacement address for search engine
-func (d *Dnsfilter) SafeSearchDomain(host string) (string, bool) {
-	val, ok := safeSearchDomains[host]
-	return val, ok
-}
-
-func (d *Dnsfilter) checkSafeSearch(host string) (Result, error) {
-	if log.GetLevel() >= log.DEBUG {
-		timer := log.StartTimer()
-		defer timer.LogElapsed("SafeSearch: lookup for %s", host)
-	}
-
-	// Check cache. Return cached result if it was found
-	cachedValue, isFound := getCachedResult(gctx.safeSearchCache, host)
-	if isFound {
-		// atomic.AddUint64(&gctx.stats.Safesearch.CacheHits, 1)
-		log.Tracef("SafeSearch: found in cache: %s", host)
-		return cachedValue, nil
-	}
-
-	safeHost, ok := d.SafeSearchDomain(host)
-	if !ok {
-		return Result{}, nil
-	}
-
-	res := Result{IsFiltered: true, Reason: FilteredSafeSearch}
-	if ip := net.ParseIP(safeHost); ip != nil {
-		res.IP = ip
-		valLen := d.setCacheResult(gctx.safeSearchCache, host, res)
-		log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, valLen)
-		return res, nil
-	}
-
-	// TODO this address should be resolved with upstream that was configured in dnsforward
-	addrs, err := net.LookupIP(safeHost)
-	if err != nil {
-		log.Tracef("SafeSearchDomain for %s was found but failed to lookup for %s cause %s", host, safeHost, err)
-		return Result{}, err
-	}
-
-	for _, i := range addrs {
-		if ipv4 := i.To4(); ipv4 != nil {
-			res.IP = ipv4
-			break
-		}
-	}
-
-	if len(res.IP) == 0 {
-		return Result{}, fmt.Errorf("no ipv4 addresses in safe search response for %s", safeHost)
-	}
-
-	// Cache result
-	valLen := d.setCacheResult(gctx.safeSearchCache, host, res)
-	log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, valLen)
-	return res, nil
-}
-
-func (d *Dnsfilter) handleSafeSearchEnable(w http.ResponseWriter, r *http.Request) {
-	d.Config.SafeSearchEnabled = true
-	d.Config.ConfigModified()
-}
-
-func (d *Dnsfilter) handleSafeSearchDisable(w http.ResponseWriter, r *http.Request) {
-	d.Config.SafeSearchEnabled = false
-	d.Config.ConfigModified()
-}
-
-func (d *Dnsfilter) handleSafeSearchStatus(w http.ResponseWriter, r *http.Request) {
-	data := map[string]interface{}{
-		"enabled": d.Config.SafeSearchEnabled,
-	}
-	jsonVal, err := json.Marshal(data)
-	if err != nil {
-		httpError(r, w, http.StatusInternalServerError, "Unable to marshal status json: %s", err)
-		return
-	}
-
-	w.Header().Set("Content-Type", "application/json")
-	_, err = w.Write(jsonVal)
-	if err != nil {
-		httpError(r, w, http.StatusInternalServerError, "Unable to write response json: %s", err)
-		return
-	}
-}

internal/dnsfilter/safesearch.go

@@ -1,5 +1,152 @@
 package dnsfilter
 
+import (
+	"bytes"
+	"encoding/binary"
+	"encoding/gob"
+	"encoding/json"
+	"fmt"
+	"net"
+	"net/http"
+	"time"
+
+	"github.com/AdguardTeam/golibs/cache"
+	"github.com/AdguardTeam/golibs/log"
+)
+
+/*
+expire byte[4]
+res Result
+*/
+func (d *Dnsfilter) setCacheResult(cache cache.Cache, host string, res Result) int {
+	var buf bytes.Buffer
+
+	expire := uint(time.Now().Unix()) + d.Config.CacheTime*60
+	exp := make([]byte, 4)
+	binary.BigEndian.PutUint32(exp, uint32(expire))
+	_, _ = buf.Write(exp)
+
+	enc := gob.NewEncoder(&buf)
+	err := enc.Encode(res)
+	if err != nil {
+		log.Error("gob.Encode(): %s", err)
+		return 0
+	}
+	val := buf.Bytes()
+	_ = cache.Set([]byte(host), val)
+	return len(val)
+}
+
+func getCachedResult(cache cache.Cache, host string) (Result, bool) {
+	data := cache.Get([]byte(host))
+	if data == nil {
+		return Result{}, false
+	}
+
+	exp := int(binary.BigEndian.Uint32(data[:4]))
+	if exp <= int(time.Now().Unix()) {
+		cache.Del([]byte(host))
+		return Result{}, false
+	}
+
+	var buf bytes.Buffer
+	buf.Write(data[4:])
+	dec := gob.NewDecoder(&buf)
+	r := Result{}
+	err := dec.Decode(&r)
+	if err != nil {
+		log.Debug("gob.Decode(): %s", err)
+		return Result{}, false
+	}
+
+	return r, true
+}
+
+// SafeSearchDomain returns replacement address for search engine
+func (d *Dnsfilter) SafeSearchDomain(host string) (string, bool) {
+	val, ok := safeSearchDomains[host]
+	return val, ok
+}
+
+func (d *Dnsfilter) checkSafeSearch(host string) (Result, error) {
+	if log.GetLevel() >= log.DEBUG {
+		timer := log.StartTimer()
+		defer timer.LogElapsed("SafeSearch: lookup for %s", host)
+	}
+
+	// Check cache. Return cached result if it was found
+	cachedValue, isFound := getCachedResult(gctx.safeSearchCache, host)
+	if isFound {
+		// atomic.AddUint64(&gctx.stats.Safesearch.CacheHits, 1)
+		log.Tracef("SafeSearch: found in cache: %s", host)
+		return cachedValue, nil
+	}
+
+	safeHost, ok := d.SafeSearchDomain(host)
+	if !ok {
+		return Result{}, nil
+	}
+
+	res := Result{IsFiltered: true, Reason: FilteredSafeSearch}
+	if ip := net.ParseIP(safeHost); ip != nil {
+		res.IP = ip
+		valLen := d.setCacheResult(gctx.safeSearchCache, host, res)
+		log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, valLen)
+		return res, nil
+	}
+
+	// TODO this address should be resolved with upstream that was configured in dnsforward
+	addrs, err := net.LookupIP(safeHost)
+	if err != nil {
+		log.Tracef("SafeSearchDomain for %s was found but failed to lookup for %s cause %s", host, safeHost, err)
+		return Result{}, err
+	}
+
+	for _, i := range addrs {
+		if ipv4 := i.To4(); ipv4 != nil {
+			res.IP = ipv4
+			break
+		}
+	}
+
+	if len(res.IP) == 0 {
+		return Result{}, fmt.Errorf("no ipv4 addresses in safe search response for %s", safeHost)
+	}
+
+	// Cache result
+	valLen := d.setCacheResult(gctx.safeSearchCache, host, res)
+	log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, valLen)
+	return res, nil
+}
+
+func (d *Dnsfilter) handleSafeSearchEnable(w http.ResponseWriter, r *http.Request) {
+	d.Config.SafeSearchEnabled = true
+	d.Config.ConfigModified()
+}
+
+func (d *Dnsfilter) handleSafeSearchDisable(w http.ResponseWriter, r *http.Request) {
+	d.Config.SafeSearchEnabled = false
+	d.Config.ConfigModified()
+}
+
+func (d *Dnsfilter) handleSafeSearchStatus(w http.ResponseWriter, r *http.Request) {
+	data := map[string]interface{}{
+		"enabled": d.Config.SafeSearchEnabled,
+	}
+	jsonVal, err := json.Marshal(data)
+	if err != nil {
+		httpError(r, w, http.StatusInternalServerError, "Unable to marshal status json: %s", err)
+		return
+	}
+
+	w.Header().Set("Content-Type", "application/json")
+	_, err = w.Write(jsonVal)
+	if err != nil {
+		httpError(r, w, http.StatusInternalServerError, "Unable to write response json: %s", err)
+		return
+	}
+}
+
 var safeSearchDomains = map[string]string{
 	"yandex.com":     "213.180.193.56",
 	"yandex.ru":      "213.180.193.56",

internal/dnsforward/access_test.go

@@ -50,7 +50,8 @@ func TestIsBlockedIPDisallowed(t *testing.T) {
 
 func TestIsBlockedIPBlockedDomain(t *testing.T) {
 	a := &accessCtx{}
-	assert.True(t, a.Init(nil, nil, []string{"host1",
+	assert.True(t, a.Init(nil, nil, []string{
+		"host1",
 		"host2",
 		"*.host.com",
 		"||host3.com^",

internal/dnsforward/msg.go

@@ -1,12 +1,12 @@
 package dnsforward
 
 import (
-	"log"
 	"net"
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/dnsfilter"
 	"github.com/AdguardTeam/dnsproxy/proxy"
+	"github.com/AdguardTeam/golibs/log"
 	"github.com/miekg/dns"
 )