From 6406d5dc57aa450215e33ab387d4b2f5226050da Mon Sep 17 00:00:00 2001 From: Oleg Obleukhov Date: Tue, 19 May 2026 03:22:40 -0700 Subject: [PATCH] Fix golangci-lint v2 issues (#525) Summary: Fix all 41 pre-existing golangci-lint v2 issues: prealloc capacity hints, lowercase error strings (ST1005), remove redundant embedded field selectors (QF1008), strings.ReplaceAll (QF1004), tagged switch (QF1003), and string conversion (QF1010). Reviewed By: pmazzini Differential Revision: D105680760 --- calnex/cmd/export.go | 2 +- calnex/verify/checks/rb.go | 2 +- cmd/ntpcheck/checker/chrony.go | 4 +-- cmd/ntpcheck/checker/stats.go | 2 +- cmd/pshark/main.go | 4 +-- cmd/ptpcheck/checker/ptp4l_test.go | 2 +- cmd/ptpcheck/cmd/diag.go | 2 +- cmd/ptpcheck/cmd/pdelay.go | 5 +--- cmd/ptpcheck/metrics/metrics_test.go | 2 +- cmd/ziffy/node/print.go | 5 ++-- cmd/ziffy/node/receiver.go | 6 ++-- cmd/ziffy/node/sender.go | 28 ++++++++--------- cmd/ziffy/node/sender_test.go | 2 +- ntp/chrony/packet_test.go | 8 ++--- ntp/control/packet_test.go | 2 +- phc/pps_source_test.go | 10 +++---- ptp/linearizability/linearizability_ptp.go | 2 +- ptp/protocol/management.go | 8 ++--- ptp/protocol/ptp4l.go | 7 +++-- ptp/protocol/ptp4l_test.go | 4 +-- ptp/protocol/tlvs.go | 8 ++--- ptp/ptp4u/server/config.go | 2 +- ptp/ptp4u/server/subscription_test.go | 12 ++++---- timestamp/timestamp.go | 12 ++++++-- timestamp/timestamp_linux.go | 35 ++++++++++++---------- timestamp/timestamp_linux_test.go | 9 +++--- 26 files changed, 94 insertions(+), 91 deletions(-) diff --git a/calnex/cmd/export.go b/calnex/cmd/export.go index 6f58ef0f..da873ef9 100644 --- a/calnex/cmd/export.go +++ b/calnex/cmd/export.go @@ -40,7 +40,7 @@ var exportCmd = &cobra.Command{ Use: "export", Short: "export calnex measurement data", Run: func(_ *cobra.Command, _ []string) { - var chs []api.Channel + chs := make([]api.Channel, 0, len(channels)) for _, channel := range channels { chs = append(chs, channel) } diff --git a/calnex/verify/checks/rb.go b/calnex/verify/checks/rb.go index ae031e95..6d6e7f18 100644 --- a/calnex/verify/checks/rb.go +++ b/calnex/verify/checks/rb.go @@ -52,7 +52,7 @@ func (p *RB) Run(target string, insecureTLS bool) error { // RBState 7 is "Hold-over (Weak GNSS)" return nil } - return fmt.Errorf("Rubidium clock state is: %s", rb.RBStateName) + return fmt.Errorf("rubidium clock state is: %s", rb.RBStateName) } // Remediate the check diff --git a/cmd/ntpcheck/checker/chrony.go b/cmd/ntpcheck/checker/chrony.go index 0cee808e..af9f01b4 100644 --- a/cmd/ntpcheck/checker/chrony.go +++ b/cmd/ntpcheck/checker/chrony.go @@ -90,7 +90,7 @@ func (n *ChronyCheck) Run() (*NTPCheckResult, error) { log.Debugf("Status: %v", packet.GetStatus()) tracking, ok := packet.(*chrony.ReplyTracking) if !ok { - return nil, fmt.Errorf("Got wrong 'tracking' response %+v", packet) + return nil, fmt.Errorf("got wrong 'tracking' response %+v", packet) } result.Correction = tracking.CurrentCorrection result.LIDesc = control.LeapDesc[uint8(tracking.LeapStatus)] @@ -106,7 +106,7 @@ func (n *ChronyCheck) Run() (*NTPCheckResult, error) { } sources, ok := packet.(*chrony.ReplySources) if !ok { - return nil, fmt.Errorf("Got wrong 'sources' response %+v", packet) + return nil, fmt.Errorf("got wrong 'sources' response %+v", packet) } log.Debugf("Got %d sources", sources.NSources) diff --git a/cmd/ntpcheck/checker/stats.go b/cmd/ntpcheck/checker/stats.go index 0f68de0d..90203bdc 100644 --- a/cmd/ntpcheck/checker/stats.go +++ b/cmd/ntpcheck/checker/stats.go @@ -85,7 +85,7 @@ func peersAverages(peers []*Peer) (*averages, error) { } func medianOffset(peers []*Peer) float64 { - offsets := []float64{} + offsets := make([]float64, 0, len(peers)) for _, p := range peers { offsets = append(offsets, p.Offset) } diff --git a/cmd/pshark/main.go b/cmd/pshark/main.go index 8d655a9a..6a880e1a 100644 --- a/cmd/pshark/main.go +++ b/cmd/pshark/main.go @@ -52,7 +52,7 @@ func (m *MultiMessageType) Set(messageType string) error { // String returns joined list of message types func (m *MultiMessageType) String() string { - s := []string{} + s := make([]string, 0, len(*m)) for _, v := range []ptp.MessageType(*m) { s = append(s, v.String()) } @@ -61,7 +61,7 @@ func (m *MultiMessageType) String() string { // GetDefaults returns default message type filter func (m *MultiMessageType) GetDefaults() []ptp.MessageType { - res := []ptp.MessageType{} + res := make([]ptp.MessageType, 0, len(ptp.MessageTypeToString)) for v := range ptp.MessageTypeToString { res = append(res, v) } diff --git a/cmd/ptpcheck/checker/ptp4l_test.go b/cmd/ptpcheck/checker/ptp4l_test.go index 5abe9a15..d1c145e6 100644 --- a/cmd/ptpcheck/checker/ptp4l_test.go +++ b/cmd/ptpcheck/checker/ptp4l_test.go @@ -373,7 +373,7 @@ func (c *fakeConn) Write(p []byte) (n int, err error) { } func prepareTestClient(t *testing.T, packets ...ptp.Packet) *ptp.MgmtClient { - outputs := []*bytes.Buffer{} + outputs := make([]*bytes.Buffer, 0, len(packets)) for _, packet := range packets { buf := &bytes.Buffer{} b, err := ptp.Bytes(packet) diff --git a/cmd/ptpcheck/cmd/diag.go b/cmd/ptpcheck/cmd/diag.go index dea45d30..5150ed46 100644 --- a/cmd/ptpcheck/cmd/diag.go +++ b/cmd/ptpcheck/cmd/diag.go @@ -180,7 +180,6 @@ func checkPathDelay(r *checker.PTPCheckResult) (status, string) { } func portServiceStatsDiagnosers(r *checker.PTPCheckResult) []diagnoser { - result := []diagnoser{} // counters are reset on ptp4l restart var maxPacketsLoss uint64 = 100 @@ -216,6 +215,7 @@ func portServiceStatsDiagnosers(r *checker.PTPCheckResult) []diagnoser { explanation: "We expect FollowUp packets to arrive in correct order", }, } + result := make([]diagnoser, 0, len(checks)) for _, check := range checks { var f diagnoser check := check // capture loop variable diff --git a/cmd/ptpcheck/cmd/pdelay.go b/cmd/ptpcheck/cmd/pdelay.go index 7a506177..6d6be303 100644 --- a/cmd/ptpcheck/cmd/pdelay.go +++ b/cmd/ptpcheck/cmd/pdelay.go @@ -300,10 +300,7 @@ func collectMulticastResponses(conn client.UDPConnWithTS, expectedSeq uint16, t1 deadline := time.Now().Add(timeout) // Set socket read timeout so ReadPacketWithRXTimestampBuf doesn't block forever - tv := unix.Timeval{ - Sec: int64(timeout.Seconds()), - Usec: (timeout % time.Second).Microseconds(), - } + tv := unix.NsecToTimeval(timeout.Nanoseconds()) if err := unix.SetsockoptTimeval(conn.ConnFd(), unix.SOL_SOCKET, unix.SO_RCVTIMEO, &tv); err != nil { log.Warningf("[pdelay] failed to set socket timeout: %v", err) } diff --git a/cmd/ptpcheck/metrics/metrics_test.go b/cmd/ptpcheck/metrics/metrics_test.go index 21e0294f..5196d241 100644 --- a/cmd/ptpcheck/metrics/metrics_test.go +++ b/cmd/ptpcheck/metrics/metrics_test.go @@ -80,7 +80,7 @@ func TestObserveOffset(t *testing.T) { // repeatNumber creates a slice with the given number repeated repetitionCount times func repeatNumber(repetitionCount int, number float64) []float64 { - slice := []float64{} + slice := make([]float64, 0, repetitionCount) for range repetitionCount { slice = append(slice, number) } diff --git a/cmd/ziffy/node/print.go b/cmd/ziffy/node/print.go index b8991035..d270ea51 100644 --- a/cmd/ziffy/node/print.go +++ b/cmd/ziffy/node/print.go @@ -238,9 +238,8 @@ func colNumber(header []string, colName string) (int, error) { } func computePrintData(sw []SwitchPrintInfo) [][]string { - ret := [][]string{ - {"uniq", "width", "hop", "ip_address", "sample_sp", "intf", "hostname", "flows", "TC", "avg_CF(ns)", "max_CF(ns)", "min_CF(ns)"}, - } + ret := make([][]string, 0, 1+len(sw)) + ret = append(ret, []string{"uniq", "width", "hop", "ip_address", "sample_sp", "intf", "hostname", "flows", "TC", "avg_CF(ns)", "max_CF(ns)", "min_CF(ns)"}) // unique counts number of devices discovered unique := 1 diff --git a/cmd/ziffy/node/receiver.go b/cmd/ziffy/node/receiver.go index 3e53261a..fad2be09 100644 --- a/cmd/ziffy/node/receiver.go +++ b/cmd/ziffy/node/receiver.go @@ -113,13 +113,13 @@ func (r *Receiver) handlePacket(rawPacket gopacket.Packet) { log.Tracef("unable to parse PTP: %v", err) return } - if ptpSync.Header.ControlField != ZiffyHexa { + if ptpSync.ControlField != ZiffyHexa { log.Trace("no ziffy packet") return } - log.Debugf("Type=%v, CF=%v, sPort=%v, sPort2=%v, sIP=%v", ptpSync.Header.MessageType().String(), - ptpSync.Header.CorrectionField, ptpSync.Header.SourcePortIdentity.PortNumber, srcPort, srcIP) + log.Debugf("Type=%v, CF=%v, sPort=%v, sPort2=%v, sIP=%v", ptpSync.MessageType().String(), + ptpSync.CorrectionField, ptpSync.SourcePortIdentity.PortNumber, srcPort, srcIP) if err := r.sendResponse(srcIP, rawPacket); err != nil { log.Tracef("unable to send response: %v", err) diff --git a/cmd/ziffy/node/sender.go b/cmd/ziffy/node/sender.go index 46ab4966..6c30bbca 100644 --- a/cmd/ziffy/node/sender.go +++ b/cmd/ziffy/node/sender.go @@ -195,7 +195,7 @@ func (s *Sender) traceRoute(destinationIP string, sendingPort int, routeID int) route := &PathInfo{switches: nil, rackSwHostname: s.rackSwHostname} ptpUDPAddr, err := net.ResolveUDPAddr("udp", net.JoinHostPort(destinationIP, fmt.Sprint(s.Config.DestinationPort))) if err != nil { - return route, fmt.Errorf("traceRoute unable to resolve UDPAddr: %w", err) + return nil, fmt.Errorf("traceRoute unable to resolve UDPAddr: %w", err) } ptpAddr := timestamp.IPToSockaddr(ptpUDPAddr.IP, s.Config.DestinationPort) domain := unix.AF_INET6 @@ -204,16 +204,16 @@ func (s *Sender) traceRoute(destinationIP string, sendingPort int, routeID int) } connFd, err := unix.Socket(domain, unix.SOCK_DGRAM, unix.IPPROTO_UDP) if err != nil { - return route, fmt.Errorf("traceRoute unable to create connection: %w", err) + return nil, fmt.Errorf("traceRoute unable to create connection: %w", err) } defer unix.Close(connFd) // set SO_REUSEPORT so we can trace network path from same source port that ptp4u uses if err = unix.SetsockoptInt(connFd, unix.SOL_SOCKET, unix.SO_REUSEPORT, 1); err != nil { - return route, fmt.Errorf("setting SO_REUSEPORT on sender socket: %w", err) + return nil, fmt.Errorf("setting SO_REUSEPORT on sender socket: %w", err) } localAddr := timestamp.IPToSockaddr(net.IPv6zero, sendingPort) if err := unix.Bind(connFd, localAddr); err != nil { - return route, fmt.Errorf("traceRoute unable to bind %v connection: %w", localAddr, err) + return nil, fmt.Errorf("traceRoute unable to bind %v connection: %w", localAddr, err) } destReached := false @@ -224,11 +224,11 @@ func (s *Sender) traceRoute(destinationIP string, sendingPort int, routeID int) for hop := s.Config.HopMin; hop <= hopMax && (!destReached || s.Config.ContReached); hop++ { for i := 0; i < s.Config.PacketsPerHop; i++ { if err := unix.SetsockoptInt(connFd, unix.IPPROTO_IPV6, unix.IPV6_UNICAST_HOPS, hop); err != nil { - return route, err + return nil, err } // First 2 bits from Traffic Class are unused, so we shift the value 2 bits if err := unix.SetsockoptInt(connFd, unix.IPPROTO_IPV6, unix.IPV6_TCLASS, s.Config.DSCP<<2); err != nil { - return route, err + return nil, err } var p ptp.Packet switch s.Config.MessageType { @@ -237,11 +237,11 @@ func (s *Sender) traceRoute(destinationIP string, sendingPort int, routeID int) case ptp.MessageSignaling: p = formSignalingPacket(hop, routeID) default: - return route, fmt.Errorf("unsupported packet type %v", s.Config.MessageType) + return nil, fmt.Errorf("unsupported packet type %v", s.Config.MessageType) } if err := s.sendEventMsg(p, connFd, ptpAddr); err != nil { - return route, err + return nil, err } select { @@ -333,13 +333,13 @@ func (s *Sender) handleIcmpPacket(rawPacket []byte, l int, rAddr net.Addr) { ) switch v := ptpPacket.(type) { case *ptp.SyncDelayReq: - corrField = v.Header.CorrectionField - sequenceID = v.Header.SequenceID - portNum = v.Header.SourcePortIdentity.PortNumber + corrField = v.CorrectionField + sequenceID = v.SequenceID + portNum = v.SourcePortIdentity.PortNumber case *ptp.Signaling: - corrField = v.Header.CorrectionField - sequenceID = v.Header.SequenceID - portNum = v.Header.SourcePortIdentity.PortNumber + corrField = v.CorrectionField + sequenceID = v.SequenceID + portNum = v.SourcePortIdentity.PortNumber default: log.Errorf("Received unexpected packet %T, ignoring", v) return diff --git a/cmd/ziffy/node/sender_test.go b/cmd/ziffy/node/sender_test.go index b6f6e5d3..d8d88b8e 100644 --- a/cmd/ziffy/node/sender_test.go +++ b/cmd/ziffy/node/sender_test.go @@ -92,7 +92,7 @@ func TestClearPaths(t *testing.T) { } func TestSortSwitchesByHop(t *testing.T) { - var switches []SwitchTrafficInfo + switches := make([]SwitchTrafficInfo, 0, 5) switches = append(switches, SwitchTrafficInfo{hop: 3, routeIdx: 1}) switches = append(switches, SwitchTrafficInfo{hop: 1, routeIdx: 2}) switches = append(switches, SwitchTrafficInfo{hop: 10, routeIdx: 5}) diff --git a/ntp/chrony/packet_test.go b/ntp/chrony/packet_test.go index 299b78d0..f57d4b1e 100644 --- a/ntp/chrony/packet_test.go +++ b/ntp/chrony/packet_test.go @@ -157,11 +157,11 @@ func TestDecodeSourceDataWithIPAddrID(t *testing.T) { require.True(t, ok, "expected *ReplySourceData") // Verify the IPAddr has Family = IPAddrID - require.Equal(t, IPAddrID, reply.SourceData.IPAddr.Family) + require.Equal(t, IPAddrID, reply.IPAddr.Family) // Verify the ID value is correctly stored in the IP field - require.Equal(t, uint8(0x00), reply.SourceData.IPAddr.IP[0]) - require.Equal(t, uint8(0x00), reply.SourceData.IPAddr.IP[1]) + require.Equal(t, uint8(0x00), reply.IPAddr.IP[0]) + require.Equal(t, uint8(0x00), reply.IPAddr.IP[1]) require.Equal(t, uint8(0x00), reply.SourceData.IPAddr.IP[2]) require.Equal(t, uint8(0x09), reply.SourceData.IPAddr.IP[3]) @@ -298,7 +298,7 @@ func TestDecodeServerStats(t *testing.T) { } require.Equal(t, want, packet) - require.Equal(t, want.ReplyHead.Reply, packet.GetType()) + require.Equal(t, want.Reply, packet.GetType()) } func TestDecodeServerStats2(t *testing.T) { diff --git a/ntp/control/packet_test.go b/ntp/control/packet_test.go index 98922cd9..81741cfa 100644 --- a/ntp/control/packet_test.go +++ b/ntp/control/packet_test.go @@ -205,7 +205,7 @@ func TestNTPControlMsg_GetAssociations(t *testing.T) { PeerEventCounter: 0, PeerEventCode: 3, } - assocData := []uint8{} + assocData := make([]uint8, 0, 8) assocData = append(assocData, uint16to2x8(1)...) assocData = append(assocData, uint16to2x8(peerStatusWord1.Word())...) assocData = append(assocData, uint16to2x8(2)...) diff --git a/phc/pps_source_test.go b/phc/pps_source_test.go index a4cb7ba2..4691ac02 100644 --- a/phc/pps_source_test.go +++ b/phc/pps_source_test.go @@ -355,8 +355,8 @@ func TestNewPiServo(t *testing.T) { servo, err := NewPiServo(time.Duration(1), time.Duration(1), time.Duration(0), mockFrequencyGetter, 0.0) require.NoError(t, err) - require.Equal(t, int64(1), servo.Servo.FirstStepThreshold) - require.Equal(t, true, servo.Servo.FirstUpdate) + require.Equal(t, int64(1), servo.FirstStepThreshold) + require.Equal(t, true, servo.FirstUpdate) require.Equal(t, -1.0, servo.MeanFreq()) require.Equal(t, "INIT", servo.GetState().String()) require.Equal(t, 3.0, servo.GetMaxFreq()) @@ -387,8 +387,8 @@ func TestNewPiServoMaxFreqError(t *testing.T) { servo, err := NewPiServo(time.Duration(1), time.Duration(1), time.Duration(0), mockFrequencyGetter, 0.0) require.NoError(t, err) - require.Equal(t, int64(1), servo.Servo.FirstStepThreshold) - require.Equal(t, true, servo.Servo.FirstUpdate) + require.Equal(t, int64(1), servo.FirstStepThreshold) + require.Equal(t, true, servo.FirstUpdate) require.Equal(t, -1.0, servo.MeanFreq()) require.Equal(t, "INIT", servo.GetState().String()) require.Equal(t, 500000.0, servo.GetMaxFreq()) @@ -472,7 +472,7 @@ func TestPPSSink_getPPSEventTimestamp(t *testing.T) { err := binary.Write(&intBuffer, hostendian.Order, &event) require.NoError(t, err) copy(buf, intBuffer.Bytes()) - fmt.Print(buf) + fmt.Print(string(buf)) }) // Act diff --git a/ptp/linearizability/linearizability_ptp.go b/ptp/linearizability/linearizability_ptp.go index 58946800..973b07fb 100644 --- a/ptp/linearizability/linearizability_ptp.go +++ b/ptp/linearizability/linearizability_ptp.go @@ -570,7 +570,7 @@ func (lt *PTPTester) RunTest(ctx context.Context) TestResult { func (lt *PTPTester) processTimestamp(sequenceID uint16, rxTimestamp time.Time) error { sendTS, found := lt.sendTS[sequenceID] if !found { - expected := []uint16{} + expected := make([]uint16, 0, len(lt.sendTS)) for e := range lt.sendTS { expected = append(expected, e) } diff --git a/ptp/protocol/management.go b/ptp/protocol/management.go index 2d9b74b1..41a672ec 100644 --- a/ptp/protocol/management.go +++ b/ptp/protocol/management.go @@ -172,20 +172,20 @@ func (p *ManagementMsgErrorStatus) UnmarshalBinary(rawBytes []byte) error { if err := binary.Read(reader, be, &p.ManagementMsgHead); err != nil { return fmt.Errorf("reading ManagementMsgErrorStatus ManagementMsgHead: %w", err) } - if err := binary.Read(reader, be, &p.ManagementErrorStatusTLV.TLVHead); err != nil { + if err := binary.Read(reader, be, &p.TLVHead); err != nil { return fmt.Errorf("reading ManagementMsgErrorStatus TLVHead: %w", err) } - if err := binary.Read(reader, be, &p.ManagementErrorStatusTLV.ManagementErrorID); err != nil { + if err := binary.Read(reader, be, &p.ManagementErrorID); err != nil { return fmt.Errorf("reading ManagementMsgErrorStatus ManagementErrorID: %w", err) } - if err := binary.Read(reader, be, &p.ManagementErrorStatusTLV.ManagementID); err != nil { + if err := binary.Read(reader, be, &p.ManagementID); err != nil { return fmt.Errorf("reading ManagementMsgErrorStatus ManagementID: %w", err) } if err := binary.Read(reader, be, &p.ManagementErrorStatusTLV.Reserved); err != nil { return fmt.Errorf("reading ManagementMsgErrorStatus Reserved: %w", err) } // packet can have trailing bytes, let's make sure we don't try to read past given length - toRead := int(p.ManagementMsgHead.Header.MessageLength) + toRead := int(p.MessageLength) toRead -= binary.Size(p.ManagementMsgHead) toRead -= binary.Size(p.ManagementErrorStatusTLV.TLVHead) toRead -= binary.Size(p.ManagementErrorStatusTLV.ManagementErrorID) diff --git a/ptp/protocol/ptp4l.go b/ptp/protocol/ptp4l.go index 0cfbbb93..28e084e4 100644 --- a/ptp/protocol/ptp4l.go +++ b/ptp/protocol/ptp4l.go @@ -223,11 +223,12 @@ func (e *UnicastMasterEntry) UnmarshalBinary(b []byte) error { e.ClockQuality.ClockClass = ClockClass(b[10]) e.ClockQuality.ClockAccuracy = ClockAccuracy(b[11]) e.ClockQuality.OffsetScaledLogVariance = binary.BigEndian.Uint16(b[12:]) - if b[14] == 0 { + switch b[14] { + case 0: e.Selected = false - } else if b[14] == 1 { + case 1: e.Selected = true - } else { + default: return fmt.Errorf("unexpected 'selected' value %d", b[14]) } e.PortState = UnicastMasterState(b[15]) diff --git a/ptp/protocol/ptp4l_test.go b/ptp/protocol/ptp4l_test.go index 5117483f..a1b30b87 100644 --- a/ptp/protocol/ptp4l_test.go +++ b/ptp/protocol/ptp4l_test.go @@ -100,7 +100,7 @@ func TestParseTimeStatusNP(t *testing.T) { func TestTimeStatusNPRequest(t *testing.T) { req := TimeStatusNPRequest() // it's normally generated from PID, set to know value - req.ManagementMsgHead.Header.SourcePortIdentity.PortNumber = 12345 + req.Header.SourcePortIdentity.PortNumber = 12345 raw, err := Bytes(req) want := []byte{ @@ -172,7 +172,7 @@ func TestParsePortStatsNP(t *testing.T) { func TestPortStatsNPRequest(t *testing.T) { req := PortStatsNPRequest() // it's normally generated from PID, set to know value - req.ManagementMsgHead.Header.SourcePortIdentity.PortNumber = 12345 + req.Header.SourcePortIdentity.PortNumber = 12345 raw, err := Bytes(req) want := []byte{ diff --git a/ptp/protocol/tlvs.go b/ptp/protocol/tlvs.go index 882124a2..047182ed 100644 --- a/ptp/protocol/tlvs.go +++ b/ptp/protocol/tlvs.go @@ -118,11 +118,7 @@ func writeTLVs(tlvs []TLV, b []byte) (int, error) { func readTLVs(tlvs []TLV, maxLength int, b []byte) ([]TLV, error) { pos := 0 var tlvType TLVType - for { - // packet can have trailing bytes, let's make sure we don't try to read past given length - if pos+tlvHeadSize > maxLength { - break - } + for pos+tlvHeadSize <= maxLength { tlvType = TLVType(binary.BigEndian.Uint16(b[pos:])) switch tlvType { @@ -353,7 +349,7 @@ func (t *PathTraceTLV) UnmarshalBinary(b []byte) error { return err } t.PathSequence = []ClockIdentity{} - for i := 0; i*8 <= int(t.TLVHead.LengthField); i++ { + for i := 0; i*8 <= int(t.LengthField); i++ { pos := tlvHeadSize + i*8 if pos+8 >= len(b) { break diff --git a/ptp/ptp4u/server/config.go b/ptp/ptp4u/server/config.go index 0743fa6c..87fa5107 100644 --- a/ptp/ptp4u/server/config.go +++ b/ptp/ptp4u/server/config.go @@ -157,7 +157,7 @@ func ReadPidFile(path string) (int, error) { return 0, err } - return strconv.Atoi(strings.Replace(string(content), "\n", "", -1)) + return strconv.Atoi(strings.ReplaceAll(string(content), "\n", "")) } // ifaceIPs gets all IPs on the specified interface diff --git a/ptp/ptp4u/server/subscription_test.go b/ptp/ptp4u/server/subscription_test.go index 964b32fb..a76aa8d5 100644 --- a/ptp/ptp4u/server/subscription_test.go +++ b/ptp/ptp4u/server/subscription_test.go @@ -110,7 +110,7 @@ func TestSubscriptionStop(t *testing.T) { require.Equal(t, 1, len(w.signalingQueue)) s := <-w.signalingQueue - require.Equal(t, ptp.TLVCancelUnicastTransmission, s.signaling.TLVs[0].(*ptp.CancelUnicastTransmissionTLV).TLVHead.TLVType) + require.Equal(t, ptp.TLVCancelUnicastTransmission, s.signaling.TLVs[0].(*ptp.CancelUnicastTransmissionTLV).TLVType) require.Equal(t, uint16(binary.Size(ptp.Header{})+binary.Size(ptp.PortIdentity{})+binary.Size(ptp.CancelUnicastTransmissionTLV{})), s.signaling.Header.MessageLength) } @@ -193,7 +193,7 @@ func TestSyncDelayReqPacket(t *testing.T) { require.Equal(t, uint16(44), sc.Sync().Header.MessageLength) // check packet length require.Equal(t, sequenceID, sc.Sync().Header.SequenceID) require.Equal(t, domainNumber, sc.Sync().Header.DomainNumber) - require.Equal(t, ptp.NewTimestamp(received), sc.Sync().SyncDelayReqBody.OriginTimestamp) + require.Equal(t, ptp.NewTimestamp(received), sc.Sync().OriginTimestamp) } func TestFollowupPacket(t *testing.T) { @@ -268,9 +268,9 @@ func TestAnnouncePacket(t *testing.T) { require.Equal(t, sequenceID+1, sc.Announce().Header.SequenceID) require.Equal(t, sp, sc.Announce().Header.SourcePortIdentity) require.Equal(t, i, sc.Announce().Header.LogMessageInterval) - require.Equal(t, ptp.ClockClass7, sc.Announce().AnnounceBody.GrandmasterClockQuality.ClockClass) - require.Equal(t, ptp.ClockAccuracyMicrosecond1, sc.Announce().AnnounceBody.GrandmasterClockQuality.ClockAccuracy) - require.Equal(t, int16(UTCOffset.Seconds()), sc.Announce().AnnounceBody.CurrentUTCOffset) + require.Equal(t, ptp.ClockClass7, sc.Announce().GrandmasterClockQuality.ClockClass) + require.Equal(t, ptp.ClockAccuracyMicrosecond1, sc.Announce().GrandmasterClockQuality.ClockAccuracy) + require.Equal(t, int16(UTCOffset.Seconds()), sc.Announce().CurrentUTCOffset) require.Equal(t, domainNumber, sc.Announce().Header.DomainNumber) } @@ -427,7 +427,7 @@ func TestSendSignalingGrant(t *testing.T) { require.Equal(t, 1, len(w.signalingQueue)) s := <-w.signalingQueue - require.Equal(t, ptp.TLVGrantUnicastTransmission, s.signaling.TLVs[0].(*ptp.GrantUnicastTransmissionTLV).TLVHead.TLVType) + require.Equal(t, ptp.TLVGrantUnicastTransmission, s.signaling.TLVs[0].(*ptp.GrantUnicastTransmissionTLV).TLVType) require.Equal(t, uint16(binary.Size(ptp.Header{})+binary.Size(ptp.PortIdentity{})+binary.Size(ptp.GrantUnicastTransmissionTLV{})), s.signaling.Header.MessageLength) } diff --git a/timestamp/timestamp.go b/timestamp/timestamp.go index 921b4300..be220548 100644 --- a/timestamp/timestamp.go +++ b/timestamp/timestamp.go @@ -143,7 +143,10 @@ func ReadPacketWithRXTimestamp(connFd int) ([]byte, unix.Sockaddr, time.Time, er oob := make([]byte, ControlSizeBytes) bbuf, sa, t, err := ReadPacketWithRXTimestampBuf(connFd, buf, oob) - return buf[:bbuf], sa, t, err + if err != nil { + return nil, nil, time.Time{}, err + } + return buf[:bbuf], sa, t, nil } // ReadPacketWithRXTimestampBuf writes byte packet into provide buffer buf, and returns number of bytes copied to the buffer, client ip and HW RX timestamp. @@ -155,7 +158,10 @@ func ReadPacketWithRXTimestampBuf(connFd int, buf, oob []byte) (int, unix.Sockad } timestamp, err := socketControlMessageTimestamp(oob, boob) - return bbuf, saddr, timestamp, err + if err != nil { + return 0, nil, time.Time{}, err + } + return bbuf, saddr, timestamp, nil } // ReadPacketWithCMsgBuf writes byte packet into provided packet and cmsg buffer, and returns number of bytes copied to the buffers @@ -164,7 +170,7 @@ func ReadPacketWithCMsgBuf(connFd int, buf, oob []byte) (int, int, unix.Sockaddr if err != nil { return 0, 0, nil, fmt.Errorf("failed to read packet: %w", err) } - return bbuf, boob, saddr, err + return bbuf, boob, saddr, nil } // ReadRXTimestamp returns HW timestamp from CMSG buffer diff --git a/timestamp/timestamp_linux.go b/timestamp/timestamp_linux.go index 8a822aa0..5c539462 100644 --- a/timestamp/timestamp_linux.go +++ b/timestamp/timestamp_linux.go @@ -81,7 +81,7 @@ func scmDataToTime(data []byte) (ts time.Time, err error) { func scmDataToSeqID(data []byte) (seqID uint32, err error) { se := (*unix.SockExtendedErr)(unsafe.Pointer(&data[0])) if unix.Errno(se.Errno) != unix.ENOMSG { - return 0, fmt.Errorf("Expected ENOMSG but got %w", unix.Errno(se.Errno)) + return 0, fmt.Errorf("expected ENOMSG but got %w", unix.Errno(se.Errno)) } return se.Data, nil } @@ -95,16 +95,16 @@ func byteToTime(data []byte) (time.Time, error) { return time.Unix(sec, nsec), nil } -func ioctlHWTimestampCaps(fd int, ifname string) (int32, int32, error) { - var rxFilter, txFilter int32 +func ioctlHWTimestampCaps(fd int, ifname string) (int32, error) { + var rxFilter int32 hw, err := unix.IoctlGetEthtoolTsInfo(fd, ifname) if err != nil { - return 0, 0, fmt.Errorf("failed to run ioctl SIOCETHTOOL to see what is supported: (%w)", err) + return 0, fmt.Errorf("failed to run ioctl SIOCETHTOOL to see what is supported: (%w)", err) } - if hw.Tx_types&(1< 0 { - txFilter = unix.HWTSTAMP_TX_ON + if hw.Tx_types&(1< 0 { @@ -115,10 +115,10 @@ func ioctlHWTimestampCaps(fd int, ifname string) (int32, int32, error) { rxFilter = unix.HWTSTAMP_FILTER_ALL } - if txFilter == 0 || rxFilter == 0 { - return rxFilter, txFilter, fmt.Errorf("hardware timestamping is not supported for the interface %s", ifname) + if rxFilter == 0 { + return 0, fmt.Errorf("hardware RX timestamping is not supported for the interface %s", ifname) } - return rxFilter, txFilter, nil + return rxFilter, nil } func ioctlTimestamp(fd int, ifname string, filter int32) error { @@ -173,7 +173,7 @@ func EnableSWTimestamps(connFd int) error { // EnableHWTimestamps enables HW timestamps (TX and RX) on the socket func EnableHWTimestamps(connFd int, iface *net.Interface) error { - rxFilter, _, err := ioctlHWTimestampCaps(connFd, iface.Name) + rxFilter, err := ioctlHWTimestampCaps(connFd, iface.Name) if err != nil { return err } @@ -202,7 +202,7 @@ func EnableHWTimestamps(connFd int, iface *net.Interface) error { // EnableHWTimestampsRx enables HW RX timestamps on the socket func EnableHWTimestampsRx(connFd int, iface *net.Interface) error { - rxFilter, _, err := ioctlHWTimestampCaps(connFd, iface.Name) + rxFilter, err := ioctlHWTimestampCaps(connFd, iface.Name) if err != nil { return err } @@ -233,7 +233,7 @@ func SeqIDSocketControlMessage(seqID uint32, soob []byte) { func waitForHWTS(connFd int) error { // Wait until TX timestamp is ready - fds := []unix.PollFd{{Fd: int32(connFd), Events: unix.POLLERR, Revents: 0}} + fds := []unix.PollFd{{Fd: int32(connFd), Events: unix.POLLERR, Revents: 0}} //nolint:gosec // fd is always small for { n, err := unix.Poll(fds, int(TimeoutTXTS.Milliseconds())) if !errors.Is(err, syscall.EINTR) { @@ -253,11 +253,16 @@ func recvoob(connFd int, oob []byte) (oobn int, err error) { var msg unix.Msghdr msg.Control = &oob[0] msg.SetControllen(len(oob)) - _, _, e1 := unix.Syscall(unix.SYS_RECVMSG, uintptr(connFd), uintptr(unsafe.Pointer(&msg)), uintptr(unix.MSG_ERRQUEUE)) + _, _, e1 := unix.Syscall(unix.SYS_RECVMSG, uintptr(connFd), uintptr(unsafe.Pointer(&msg)), uintptr(unix.MSG_ERRQUEUE)) //nolint:gosec // fd is always small if e1 != 0 { return 0, e1 } - return int(msg.Controllen), nil + // Controllen is bounded by len(oob) we passed to SetControllen + n := msg.Controllen + if n > uint64(len(oob)) { + n = uint64(len(oob)) + } + return int(n), nil } // socketControlMessageTimestamp is a very optimised version of ParseSocketControlMessage @@ -427,7 +432,7 @@ func EnableTimestamps(ts Timestamp, connFd int, iface *net.Interface) error { return fmt.Errorf("cannot enable software rx timestamps: %w", err) } default: - return fmt.Errorf("Unrecognized timestamp type: %s", ts) + return fmt.Errorf("unrecognized timestamp type: %s", ts) } return nil } diff --git a/timestamp/timestamp_linux_test.go b/timestamp/timestamp_linux_test.go index c7302336..3d62533f 100644 --- a/timestamp/timestamp_linux_test.go +++ b/timestamp/timestamp_linux_test.go @@ -230,7 +230,7 @@ func TestEnableTimestamps(t *testing.T) { // Unsupported err = EnableTimestamps(42, connFd, &net.Interface{Name: "lo", Index: 1}) - require.Equal(t, fmt.Errorf("Unrecognized timestamp type: Unsupported"), err) + require.Equal(t, fmt.Errorf("unrecognized timestamp type: Unsupported"), err) } func TestSocketControlMessageTimestamp(t *testing.T) { @@ -368,10 +368,9 @@ func TestReadHWTimestampCaps(t *testing.T) { connFd, err := ConnFd(conn) require.NoError(t, err) - rxFilters, txType, err := ioctlHWTimestampCaps(connFd, "lo") + rxFilters, err := ioctlHWTimestampCaps(connFd, "lo") require.Error(t, err) - // hw timestamps are disabled for lo - require.Equal(t, int32(0), txType) + require.Contains(t, err.Error(), "not supported") require.Equal(t, int32(0), rxFilters) } @@ -394,7 +393,7 @@ func TestScmDataToSeqIDErrornoNotENOMSG(t *testing.T) { } _, err := scmDataToSeqID(hwData) require.Error(t, err) - require.ErrorContains(t, err, "Expected ENOMSG but got function not implemented") + require.ErrorContains(t, err, "expected ENOMSG but got function not implemented") } func TestSeqIDSocketControlMessage(t *testing.T) {