Add gocritic and fix some of the issues

Some of the larger if statements have been marked with 'nolint' as they are correct and concise. This makes it hard to write the same functionality in a better way.
dmachard · Nov 20, 2023 · a07baa0 · a07baa0
1 parent 3b53cc7
commit a07baa0
Show file tree

Hide file tree

Showing 26 changed files with 87 additions and 103 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -22,7 +22,7 @@ linters:
   - gosimple
   - ineffassign
   - stylecheck
+  - gocritic
 
   # list of linters to use in the future:
-  #- gocritic
   #- gosec
diff --git a/collectors/dnstap.go b/collectors/dnstap.go
@@ -80,13 +80,12 @@ func (c *Dnstap) ReadConfig() {
 	}
 
 	c.sockPath = c.config.Collectors.Dnstap.SockPath
+	c.connMode = "tcp"
 
 	if len(c.config.Collectors.Dnstap.SockPath) > 0 {
 		c.connMode = "unix"
 	} else if c.config.Collectors.Dnstap.TLSSupport {
 		c.connMode = "tls"
-	} else {
-		c.connMode = "tcp"
 	}
 }
 

diff --git a/collectors/file_ingestor.go b/collectors/file_ingestor.go
@@ -281,8 +281,6 @@ func (c *FileIngestor) ProcessPcap(filePath string) {
 
 	}
 
-	// remove it ?
-	//assembler.FlushAll()
 	c.LogInfo("pcap file [%s] processing terminated, %d packet(s) read", fileName, nbPackets)
 
 	// remove it ?
@@ -291,7 +289,7 @@ func (c *FileIngestor) ProcessPcap(filePath string) {
 		os.Remove(filePath)
 	}
 
-	//close chan
+	// close chan
 	close(fragIP4Chan)
 	close(fragIP6Chan)
 	close(udpChan)

diff --git a/collectors/file_tail_test.go b/collectors/file_tail_test.go
@@ -30,7 +30,7 @@ func TestTailRun(t *testing.T) {
 	g := loggers.NewFakeLogger()
 	c := NewTail([]dnsutils.Worker{g}, config, logger.New(false), "test")
 	if err := c.Follow(); err != nil {
-		log.Fatal("collector tail following error: ", err)
+		t.Errorf("collector tail following error: %e", err)
 	}
 	go c.Run()
 
@@ -39,7 +39,7 @@ func TestTailRun(t *testing.T) {
 	w := bufio.NewWriter(tmpFile)
 	linesToWrite := "2021-08-27T07:18:35.775473Z dnscollector CLIENT_QUERY NOERROR 192.168.1.5 45660 INET INET 43b www.google.org A 0.00000"
 	if _, err := w.WriteString(linesToWrite + "\n"); err != nil {
-		log.Fatal("Failed to write to temporary file", err)
+		t.Errorf("Failed to write to temporary file: %e", err)
 	}
 	w.Flush()
 

diff --git a/collectors/sniffer_afpacket.go b/collectors/sniffer_afpacket.go
@@ -273,7 +273,7 @@ func (c *AfpacketSniffer) Run() {
 	if c.fd == 0 {
 		if err := c.Listen(); err != nil {
 			c.LogError("init raw socket failed: %v\n", err)
-			os.Exit(1)
+			os.Exit(1) // nolint
 		}
 	}
 
@@ -348,7 +348,7 @@ func (c *AfpacketSniffer) Run() {
 		oob := make([]byte, 100)
 
 		for {
-			//flags, from
+			// flags, from
 			bufN, oobn, _, _, err := syscall.Recvmsg(c.fd, buf, oob, 0)
 			if err != nil {
 				if errors.Is(err, syscall.EINTR) {

diff --git a/collectors/sniffer_xdp.go b/collectors/sniffer_xdp.go
@@ -140,7 +140,7 @@ func (c *XDPSniffer) Run() {
 	})
 	if err != nil {
 		c.LogError("could not attach XDP program: %s", err)
-		os.Exit(1)
+		os.Exit(1) // nolint
 	}
 	defer l.Close()
 
@@ -168,7 +168,7 @@ func (c *XDPSniffer) Run() {
 				// send the config to the dns processor
 				dnsProcessor.ConfigChan <- cfg
 
-			//dns message to read ?
+			// dns message to read ?
 			case dm := <-dnsChan:
 
 				// update identity with config ?

diff --git a/collectors/tzsp.go b/collectors/tzsp.go
@@ -133,7 +133,7 @@ func (c *TZSPSniffer) Run() {
 		buf := make([]byte, 65536)
 		oob := make([]byte, 100)
 		for {
-			//flags, from
+			// flags, from
 			bufN, oobn, _, _, err := c.listen.ReadMsgUDPAddrPort(buf, oob)
 			if err != nil {
 				panic(err)

diff --git a/dnsutils/config.go b/dnsutils/config.go
@@ -419,7 +419,7 @@ type Config struct {
 			ConnectTimeout    int    `yaml:"connect-timeout"`
 			Transport         string `yaml:"transport"`
 			FlushInterval     int    `yaml:"flush-interval"`
-			TLSSupport        bool   `yaml:"tls-support"` //deprecated
+			TLSSupport        bool   `yaml:"tls-support"` // deprecated
 			TLSInsecure       bool   `yaml:"tls-insecure"`
 			TLSMinVersion     string `yaml:"tls-min-version"`
 			CAFile            string `yaml:"ca-file"`

diff --git a/dnsutils/dns_parser.go b/dnsutils/dns_parser.go
@@ -279,7 +279,7 @@ func DecodePayload(dm *DNSMessage, header *DNSHeader, config *Config) error {
 	// decode DNS answers
 	if header.Ancount > 0 {
 		answers, offset, err := DecodeAnswer(header.Ancount, payloadOffset, dm.DNS.Payload)
-		if err == nil {
+		if err == nil { // nolint
 			dm.DNS.DNSRRs.Answers = answers
 			payloadOffset = offset
 		} else if dm.DNS.Flags.TC && (errors.Is(err, ErrDecodeDNSAnswerTooShort) || errors.Is(err, ErrDecodeDNSAnswerRdataTooShort) || errors.Is(err, ErrDecodeDNSLabelTooShort)) {
@@ -309,7 +309,7 @@ func DecodePayload(dm *DNSMessage, header *DNSHeader, config *Config) error {
 	if header.Arcount > 0 {
 		// decode additional answers
 		answers, _, err := DecodeAnswer(header.Arcount, payloadOffset, dm.DNS.Payload)
-		if err == nil {
+		if err == nil { // nolint
 			dm.DNS.DNSRRs.Records = answers
 		} else if dm.DNS.Flags.TC && (errors.Is(err, ErrDecodeDNSAnswerTooShort) || errors.Is(err, ErrDecodeDNSAnswerRdataTooShort) || errors.Is(err, ErrDecodeDNSLabelTooShort)) {
 			dm.DNS.MalformedPacket = true
@@ -320,7 +320,7 @@ func DecodePayload(dm *DNSMessage, header *DNSHeader, config *Config) error {
 		}
 		// decode EDNS options, if there are any
 		edns, _, err := DecodeEDNS(header.Arcount, payloadOffset, dm.DNS.Payload)
-		if err == nil {
+		if err == nil { // nolint
 			dm.EDNS = edns
 		} else if dm.DNS.Flags.TC && (errors.Is(err, ErrDecodeDNSAnswerTooShort) ||
 			errors.Is(err, ErrDecodeDNSAnswerRdataTooShort) ||
@@ -488,7 +488,7 @@ func ParseLabels(offset int, payload []byte) (string, int, error) {
 		}
 
 		length := int(payload[offset])
-		if length == 0 {
+		if length == 0 { // nolint
 			if endOffset == -1 {
 				endOffset = offset + 1
 			}
@@ -532,7 +532,7 @@ func ParseLabels(offset int, payload []byte) (string, int, error) {
 		}
 	}
 
-	return strings.Join(labels[:], "."), endOffset, nil
+	return strings.Join(labels, "."), endOffset, nil
 }
 
 func ParseRdata(rdatatype string, rdata []byte, payload []byte, rdataOffset int) (string, error) {
@@ -888,7 +888,7 @@ func ParseSVCParam(svcParamKey uint16, paramData []byte) (string, error) {
 		}
 		return "", nil
 	case 3:
-		//port
+		// port
 		if len(paramData) != 2 {
 			return "", ErrDecodeDNSAnswerRdataTooShort
 		}

diff --git a/dnsutils/dns_parser_test.go b/dnsutils/dns_parser_test.go
@@ -848,7 +848,7 @@ func TestDecodeQuestion_SkipOpt(t *testing.T) {
 		0x00, 0x00, 0x0e, 0x10,
 		// RDLENGTH
 		0x00, 0x01,
-		//RDATA
+		// RDATA
 		0x01,
 		// 2nd resource record
 		0x0f, 0x64,
@@ -1331,7 +1331,7 @@ func TestDecodeDnsLabel_EndOffset_WithPtr(t *testing.T) {
 
 func TestDecodePayload_QueryHappy(t *testing.T) {
 	payload := []byte{
-		//header
+		// header
 		0x9e, 0x84, 0x01, 0x20, 0x00, 0x01, 0x00, 0x00,
 		0x00, 0x00, 0x00, 0x01,
 		// query section
@@ -1397,7 +1397,7 @@ func TestDecodePayload_QueryHappy(t *testing.T) {
 }
 func TestDecodePayload_QueryInvalid(t *testing.T) {
 	payload := []byte{
-		//header
+		// header
 		0x9e, 0x84, 0x01, 0x20, 0x00, 0x01, 0x00, 0x00,
 		0x00, 0x00, 0x00, 0x01,
 		// query section

diff --git a/dnsutils/edns_parser.go b/dnsutils/edns_parser.go
@@ -122,7 +122,7 @@ func DecodeEDNS(arcount int, startOffset int, payload []byte) (DNSExtended, int,
 			   /                                                               /
 			   +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ */
 			endOffset := offsetNext + 10 + int(rdlength)
-			offsetNext = offsetNext + 10
+			offsetNext += 10
 
 			for {
 				// no more options to read ?

diff --git a/dnsutils/edns_parser_test.go b/dnsutils/edns_parser_test.go
@@ -57,7 +57,8 @@ func TestDecodeReply_EDNS(t *testing.T) {
 	o := &dns.EDNS0_COOKIE{Code: 10, Cookie: "aaaa"}
 	e.Option = append(e.Option, o)
 
-	m.Extra = append(dm.Extra, e)
+	m.Extra = dm.Extra
+	m.Extra = append(m.Extra, e)
 
 	m.SetRcode(dm, 42) // 32(extended rcode) + 10(rcode)
 
@@ -92,7 +93,7 @@ func TestDecodeQuery_EdnsSubnet(t *testing.T) {
 		0x00, 0x00, 0x80, 0x00,
 		// RDLENGTH
 		0x00, 0x0b,
-		//RDATA
+		// RDATA
 		// CODE - Client subnet
 		0x00, 0x08,
 		// Length
@@ -162,7 +163,7 @@ func TestDecodeQuery_EdnsSubnetV6(t *testing.T) {
 		0x00, 0x00, 0x80, 0x00,
 		// RDLENGTH
 		0x00, 0x0b,
-		//RDATA
+		// RDATA
 		// CODE - Client subnet
 		0x00, 0x08,
 		// Length
@@ -233,7 +234,7 @@ func TestDecodeQuery_EdnsSubnet_invalidFam(t *testing.T) {
 		0x00, 0x00, 0x80, 0x00,
 		// RDLENGTH
 		0x00, 0x0b,
-		//RDATA
+		// RDATA
 		// CODE - Client subnet
 		0x00, 0x08,
 		// Length
@@ -291,7 +292,7 @@ func TestDecodeQuery_EdnsSubnet_Short(t *testing.T) {
 		0x00, 0x00, 0x80, 0x00,
 		// RDLENGTH
 		0x00, 0x06,
-		//RDATA
+		// RDATA
 		// CODE - Client subnet
 		0x00, 0x08,
 		// Length
@@ -349,15 +350,15 @@ func TestDecodeQuery_EdnsSubnet_NoAddr(t *testing.T) {
 		0x00, 0x00, 0x80, 0x00,
 		// RDLENGTH
 		0x00, 0x08,
-		//RDATA
+		// RDATA
 		// CODE - Client subnet
 		0x00, 0x08,
 		// Length
 		0x00, 0x04,
 		// Option data
 		// family
 		0x00, 0x01,
-		//prefix-len
+		// prefix-len
 		0x18,
 		// scope prefix-len
 		0x00,
@@ -418,7 +419,7 @@ func TestDecodeAnswer_EdnsError(t *testing.T) {
 		0x00, 0x00, 0x80, 0x00,
 		// RDLENGTH
 		0x00, 0x06,
-		//RDATA
+		// RDATA
 		// CODE - Extended error
 		0x00, 0x0f,
 		// Length
@@ -472,7 +473,7 @@ func TestDecodeAnswer_EdnsErrorText(t *testing.T) {
 		0x00, 0x00, 0x80, 0x00,
 		// RDLENGTH
 		0x00, 0x0c,
-		//RDATA
+		// RDATA
 		// CODE - Extended error
 		0x00, 0x0f,
 		// Length
@@ -529,7 +530,7 @@ func TestDecodeAnswer_EdnsErrorShort(t *testing.T) {
 		0x00, 0x00, 0x80, 0x00,
 		// RDLENGTH
 		0x00, 0x05,
-		//RDATA
+		// RDATA
 		// CODE - Extended error
 		0x00, 0x0f,
 		// Length
@@ -579,7 +580,7 @@ func TestDecodeEdns_Short(t *testing.T) {
 				0x00, 0x00, 0x80, 0x00,
 				// RDLENGTH
 				0x00, 0x10,
-				//RDATA
+				// RDATA
 				// CODE - Extended error
 				0x00, 0x0f,
 				// Length
@@ -602,7 +603,7 @@ func TestDecodeEdns_Short(t *testing.T) {
 				0x00, 0x00, 0x80, 0x00,
 				// RDLENGTH
 				0x00, 0x02,
-				//RDATA
+				// RDATA
 				// CODE - Extended error
 				0x00, 0x0f,
 			},
@@ -621,7 +622,7 @@ func TestDecodeEdns_Short(t *testing.T) {
 				0x00, 0x00, 0x80, 0x00,
 				// RDLENGTH
 				0x00, 0x05,
-				//RDATA
+				// RDATA
 				// CODE - Extended error
 				0x00, 0x0f,
 				// Length
@@ -644,7 +645,7 @@ func TestDecodeEdns_Short(t *testing.T) {
 				0x00, 0x00, 0x80, 0x00,
 				// RDLENGTH
 				0x00, 0x05,
-				//RDATA
+				// RDATA
 				// CODE - Extended error
 				0x00, 0x0f,
 				// Length
@@ -663,7 +664,7 @@ func TestDecodeEdns_Short(t *testing.T) {
 				0x00, 0x00, 0x80, 0x00,
 				// RDLENGTH
 				0x00, 0x07,
-				//RDATA
+				// RDATA
 				// CODE - Extended error
 				0x00, 0x0f,
 				// Length
@@ -686,7 +687,7 @@ func TestDecodeEdns_Short(t *testing.T) {
 				0x00, 0x00, 0x80, 0x00,
 				// RDLENGTH
 				0x00, 0x06,
-				//RDATA
+				// RDATA
 				// CODE - Extended error
 				0x00, 0x0f,
 				// Length
@@ -720,7 +721,7 @@ func TestDecodeEdns_MultipleOpts(t *testing.T) {
 		0x00, 0x00, 0x80, 0x00,
 		// RDLENGTH
 		0x00, 0x06,
-		//RDATA
+		// RDATA
 		// CODE - Extended error
 		0x00, 0x0f,
 		// Length
@@ -737,7 +738,7 @@ func TestDecodeEdns_MultipleOpts(t *testing.T) {
 		0x00, 0x00, 0x00, 0x01,
 		// RDLENGTH
 		0x00, 0x06,
-		//RDATA
+		// RDATA
 		// CODE - Extended error
 		0x00, 0x0f,
 		// Length