Fix issues reported by golangsci-lint

Author: Danielius1922

.github/workflows/test-with-cfg.yml

@@ -78,6 +78,8 @@ jobs:
         uses: actions/upload-artifact@v4
         with:
           name: ${{ steps.coverage.outputs.directory }}
+          # match bridge.coverage.txt,  bridge.unit.coverage.txt, coverage.txt, pkg.unit.coverage.txt and schema.unit.coverage.txt
           path: .tmp/*coverage.txt
+          include-hidden-files: true
           if-no-files-found: error
           retention-days: 1

.golangci.yml

@@ -27,7 +27,7 @@ linters:
     - asciicheck       # Simple linter to check that your code does not contain non-ASCII identifiers
     - bidichk          # Checks for dangerous unicode character sequences
     - bodyclose        # Checks whether HTTP response body is closed successfully
-    # - copyloopvar      # Detects places where loop variables are copied
+    - copyloopvar      # Detects places where loop variables are copied
     - decorder         # Check declaration order and count of types, constants, variables and functions
     - dogsled          # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f())
     - dupl             # Tool for code clone detection
@@ -37,7 +37,6 @@ linters:
     - errchkjson       # Checks types passed to the json encoding functions. Reports unsupported types and optionally reports occasions, where the check for the returned error can be omitted.
     - errname          # Checks that sentinel errors are prefixed with the `Err` and error types are suffixed with the `Error`.
     - errorlint        # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.
-    - exportloopref    # checks for pointers to enclosing loop variables
     - forcetypeassert  # finds forced type assertions
     - gci              # Gci control golang package import order and make it always deterministic.
     - gocheckcompilerdirectives  # Checks that go compiler directive comments (//go:) are valid.

client/client.go

@@ -28,6 +28,7 @@ import (
 	"github.com/pion/dtls/v2"
 	"github.com/plgd-dev/device/v2/client/core"
 	"github.com/plgd-dev/device/v2/client/core/otm"
+	"github.com/plgd-dev/device/v2/internal/math"
 	"github.com/plgd-dev/device/v2/pkg/log"
 	"github.com/plgd-dev/device/v2/pkg/net/coap"
 	"github.com/plgd-dev/go-coap/v3/net/blockwise"
@@ -51,7 +52,7 @@ type subscription = interface {
 }
 
 type Config struct {
-	DeviceCacheExpirationSeconds   int64
+	DeviceCacheExpirationSeconds   uint64
 	ObserverPollingIntervalSeconds uint64 // 0 means 3 seconds
 	ObserverFailureThreshold       uint8  // 0 means 3
 
@@ -67,16 +68,27 @@ type Config struct {
 	DeviceOwnershipBackend *DeviceOwnershipBackendConfig `yaml:",omitempty"`
 }
 
+func toDuration(seconds uint64, def time.Duration) (time.Duration, error) {
+	if seconds == 0 {
+		return def, nil
+	}
+	const maxDurationSeconds uint64 = (1<<63 - 1) / uint64(time.Second)
+	if seconds > maxDurationSeconds {
+		return 0, errors.New("invalid value: interval overflows maximal duration")
+	}
+	return math.CastTo[time.Duration](seconds * uint64(time.Second)), nil
+}
+
 // NewClientFromConfig constructs a new local client from the proto configuration.
 func NewClientFromConfig(cfg *Config, app ApplicationCallback, logger core.Logger) (*Client, error) {
-	var cacheExpiration time.Duration
-	if cfg.DeviceCacheExpirationSeconds > 0 {
-		cacheExpiration = time.Second * time.Duration(cfg.DeviceCacheExpirationSeconds)
+	cacheExpiration, err := toDuration(cfg.DeviceCacheExpirationSeconds, 0)
+	if err != nil {
+		return nil, errors.New("invalid DeviceCacheExpirationSeconds value")
 	}
 
-	observerPollingInterval := time.Second * 3
-	if cfg.ObserverPollingIntervalSeconds > 0 {
-		observerPollingInterval = time.Second * time.Duration(cfg.ObserverPollingIntervalSeconds)
+	observerPollingInterval, err := toDuration(cfg.ObserverPollingIntervalSeconds, time.Second*3)
+	if err != nil {
+		return nil, errors.New("invalid ObserverPollingIntervalSeconds value")
 	}
 
 	tcpDialOpts := make([]tcp.Option, 0, 5)
@@ -93,20 +105,20 @@ func NewClientFromConfig(cfg *Config, app ApplicationCallback, logger core.Logge
 	tcpDialOpts = append(tcpDialOpts, options.WithErrors(errFn))
 	udpDialOpts = append(udpDialOpts, options.WithErrors(errFn))
 
-	keepAliveConnectionTimeout := time.Second * 60
-	if cfg.KeepAliveConnectionTimeoutSeconds > 0 {
-		keepAliveConnectionTimeout = time.Second * time.Duration(cfg.KeepAliveConnectionTimeoutSeconds)
+	keepAliveConnectionTimeout, err := toDuration(cfg.KeepAliveConnectionTimeoutSeconds, time.Second*60)
+	if err != nil {
+		return nil, errors.New("invalid KeepAliveConnectionTimeoutSeconds value")
 	}
 	tcpDialOpts = append(tcpDialOpts, options.WithKeepAlive(3, keepAliveConnectionTimeout/3, func(cc *tcpClient.Conn) {
-		errFn(fmt.Errorf("keepalive failed for tcp: %v", cc.RemoteAddr()))
-		if err := cc.Close(); err != nil {
-			errFn(fmt.Errorf("failed to close tcp connection: %v", cc.RemoteAddr()))
+		errFn(fmt.Errorf("keepalive failed for tcp %v", cc.RemoteAddr()))
+		if errC := cc.Close(); errC != nil {
+			errFn(fmt.Errorf("failed to close tcp connection %v: %w", cc.RemoteAddr(), errC))
 		}
 	}))
 	udpDialOpts = append(udpDialOpts, options.WithKeepAlive(3, keepAliveConnectionTimeout/3, func(cc *udpClient.Conn) {
-		errFn(fmt.Errorf("keepalive failed for udp: %v", cc.RemoteAddr()))
-		if err := cc.Close(); err != nil {
-			errFn(fmt.Errorf("failed to close udp connection: %v", cc.RemoteAddr()))
+		errFn(fmt.Errorf("keepalive failed for udp %v", cc.RemoteAddr()))
+		if errC := cc.Close(); errC != nil {
+			errFn(fmt.Errorf("failed to close udp connection %v: %w", cc.RemoteAddr(), errC))
 		}
 	}))
 
@@ -121,10 +133,11 @@ func NewClientFromConfig(cfg *Config, app ApplicationCallback, logger core.Logge
 		tcpDialOpts = append(tcpDialOpts, options.WithDisablePeerTCPSignalMessageCSMs())
 	}
 
-	defaultTransferDuration := time.Second * 15
-	if cfg.DefaultTransferDurationSeconds > 0 {
-		defaultTransferDuration = time.Second * time.Duration(cfg.DefaultTransferDurationSeconds)
+	defaultTransferDuration, err := toDuration(cfg.DefaultTransferDurationSeconds, time.Second*15)
+	if err != nil {
+		return nil, errors.New("invalid DefaultTransferDurationSeconds value")
 	}
+
 	tcpDialOpts = append(tcpDialOpts, options.WithBlockwise(true, blockwise.SZX1024, defaultTransferDuration))
 	udpDialOpts = append(udpDialOpts, options.WithBlockwise(true, blockwise.SZX1024, defaultTransferDuration))
 

client/core/discover.go

@@ -23,6 +23,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/plgd-dev/device/v2/internal/math"
 	"github.com/plgd-dev/device/v2/pkg/net/coap"
 	"github.com/plgd-dev/go-coap/v3/message"
 	"github.com/plgd-dev/go-coap/v3/message/pool"
@@ -128,7 +129,7 @@ func DialDiscoveryAddresses(ctx context.Context, cfg DiscoveryConfiguration, err
 	// We need to separate messageIDs for upd4 and udp6, because if any docker container has isolated network
 	// iotivity-lite gets error EINVAL(22) for sendmsg with UDP6 for some interfaces. If it happens, the device is
 	// not discovered and msgid is cached so all other multicast messages from another interfaces are dropped for deduplication.
-	msgIDudp4 := uint16(message.GetMID())
+	msgIDudp4 := math.CastTo[uint16](message.GetMID())
 	msgIDudp6 := msgIDudp4 + ^uint16(0)/2
 
 	for _, address := range cfg.MulticastAddressUDP4 {

go.mod

@@ -22,6 +22,7 @@ require (
 	github.com/ugorji/go/codec v1.2.12
 	github.com/web-of-things-open-source/thingdescription-go v0.0.0-20240513190706-79b5f39190eb
 	go.uber.org/atomic v1.11.0
+	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
 	golang.org/x/sync v0.8.0
 	google.golang.org/grpc v1.65.0
 	gopkg.in/yaml.v3 v3.0.1
@@ -36,7 +37,6 @@ require (
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/x448/float16 v0.8.4 // indirect
 	golang.org/x/crypto v0.25.0 // indirect
-	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
 	golang.org/x/net v0.27.0 // indirect
 	golang.org/x/sys v0.23.0 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240730163845-b1a4ccb954bf // indirect

internal/math/cast.go

@@ -0,0 +1,7 @@
+package math
+
+import "golang.org/x/exp/constraints"
+
+func CastTo[T, F constraints.Integer](from F) T {
+	return T(from)
+}

schema/credential/credential.go

@@ -86,7 +86,7 @@ func (c CredentialType) String() string {
 		c &^= CredentialType_ASYMMETRIC_ENCRYPTION_KEY
 	}
 	if c != 0 {
-		res = append(res, fmt.Sprintf("unknown(%v)", int(c)))
+		res = append(res, fmt.Sprintf("unknown(%v)", uint8(c)))
 	}
 	return strings.Join(res, "|")
 }

schema/platform/platform.go

@@ -18,7 +18,11 @@
 // https://github.com/openconnectivityfoundation/core/blob/master/swagger2.0/oic.wk.p.swagger.json
 package platform
 
-import "fmt"
+import (
+	"fmt"
+
+	"github.com/plgd-dev/device/v2/internal/math"
+)
 
 const (
 	ResourceType = "oic.wk.p"
@@ -42,10 +46,10 @@ type Platform struct {
 }
 
 func (p Platform) GetVersion() (uint8, uint8, uint8, uint8) {
-	major := uint8((p.Version >> 24) & 0xFF)
-	minor := uint8((p.Version >> 16) & 0xFF)
-	patch := uint8((p.Version >> 8) & 0xFF)
-	bugfix := uint8(p.Version & 0xFF)
+	major := math.CastTo[uint8]((p.Version >> 24) & 0xFF)
+	minor := math.CastTo[uint8]((p.Version >> 16) & 0xFF)
+	patch := math.CastTo[uint8]((p.Version >> 8) & 0xFF)
+	bugfix := math.CastTo[uint8](p.Version & 0xFF)
 	return major, minor, patch, bugfix
 }
 

schema/pstat/pstat.go

@@ -115,7 +115,7 @@ func (m OperationalMode) String() string {
 		m &^= OperationalMode_CLIENT_DIRECTED
 	}
 	if m != 0 {
-		res = append(res, fmt.Sprintf("unknown(%v)", int(m)))
+		res = append(res, fmt.Sprintf("unknown(%v)", uint8(m)))
 	}
 	return strings.Join(res, "|")
 }
@@ -145,7 +145,7 @@ func (m ProvisioningMode) String() string {
 		m &^= ProvisioningMode_INIT_SEC_SOFT_UPDATE
 	}
 	if m != 0 {
-		res = append(res, fmt.Sprintf("unknown(%v)", int(m)))
+		res = append(res, fmt.Sprintf("unknown(%v)", uint16(m)))
 	}
 	return strings.Join(res, "|")
 }