Fix VNC lint, 386 atomic alignment, and Sonar code smells

client/vnc/server/agent_ipc.go

@@ -1,4 +1,4 @@
-//go:build !js && !ios && !android
+//go:build darwin || windows
 
 package server
 

client/vnc/server/cursor_windows.go

@@ -259,43 +259,50 @@ func decodeColorCursor(hbmColor, hbmMask windows.Handle) (*image.RGBA, error) {
 	if hbmMask != 0 {
 		mask, _ = dibCopy(hbmMask, w, h)
 	}
+	hasAlpha := colorHasAlpha(color)
 	img := image.NewRGBA(image.Rect(0, 0, int(w), int(h)))
-	hasAlpha := false
-	for i := 0; i < len(color); i += 4 {
-		if color[i+3] != 0 {
-			hasAlpha = true
-			break
-		}
-	}
 	for y := int32(0); y < h; y++ {
 		for x := int32(0); x < w; x++ {
 			si := (y*w + x) * 4
-			di := (y*w + x) * 4
 			b := color[si]
 			g := color[si+1]
 			r := color[si+2]
-			a := color[si+3]
+			a := pixelAlpha(color[si+3], si, mask, hasAlpha)
-			if !hasAlpha {
+			img.Pix[si+0] = r
-				a = 255
+			img.Pix[si+1] = g
-				if mask != nil {
+			img.Pix[si+2] = b
-					// AND mask: 1 = transparent, 0 = opaque. The DIB
+			img.Pix[si+3] = a
-					// representation we requested is 32bpp so each "bit"
-					// is a 4-byte entry; we use the first byte as the
-					// effective AND value.
-					if mask[si] != 0 {
-						a = 0
-					}
-				}
-			}
-			img.Pix[di+0] = r
-			img.Pix[di+1] = g
-			img.Pix[di+2] = b
-			img.Pix[di+3] = a
 		}
 	}
 	return img, nil
 }
 
+// colorHasAlpha reports whether any pixel of a 32bpp BGRA buffer has a
+// non-zero alpha. Cursors authored without alpha leave the channel at 0
+// and rely on hbmMask for transparency.
+func colorHasAlpha(color []byte) bool {
+	for i := 0; i < len(color); i += 4 {
+		if color[i+3] != 0 {
+			return true
+		}
+	}
+	return false
+}
+
+// pixelAlpha returns the effective alpha for a colour-cursor pixel. When
+// the source bitmap already has alpha we trust it; otherwise the AND mask
+// decides (1 = transparent, 0 = opaque). The 32bpp DIB stores each AND
+// bit as a 4-byte entry; the first byte carries the effective value.
+func pixelAlpha(colorA byte, si int32, mask []byte, hasAlpha bool) byte {
+	if hasAlpha {
+		return colorA
+	}
+	if mask != nil && mask[si] != 0 {
+		return 0
+	}
+	return 255
+}
+
 // decodeMonoCursor handles legacy 1bpp cursors where hbmMask is twice as
 // tall as the visible sprite: rows [0..h) are the AND mask and rows [h..2h)
 // are the XOR mask. We render the visible half into RGBA, treating

client/vnc/server/metrics_conn.go

@@ -41,15 +41,15 @@ type metricsConn struct {
 
 	recorder func(SessionTick)
 
-	bytesOut    uint64
+	bytesOut    atomic.Uint64
-	writes      uint64
+	writes      atomic.Uint64
-	writeNanos  uint64
+	writeNanos  atomic.Uint64
-	largestPkt  uint64
+	largestPkt  atomic.Uint64
-	fbus        uint64
+	fbus        atomic.Uint64
-	fbuBytes    uint64
+	fbuBytes    atomic.Uint64
-	fbuRects    uint64
+	fbuRects    atomic.Uint64
-	maxFBUBytes uint64
+	maxFBUBytes atomic.Uint64
-	maxFBURects uint64
+	maxFBURects atomic.Uint64
 
 	tickMu     sync.Mutex
 	tickStart  time.Time
@@ -104,10 +104,10 @@ func (m *metricsConn) flushTick(final bool) {
 	m.tickMu.Lock()
 	defer m.tickMu.Unlock()
 
-	b := atomic.LoadUint64(&m.bytesOut)
+	b := m.bytesOut.Load()
-	w := atomic.LoadUint64(&m.writes)
+	w := m.writes.Load()
-	f := atomic.LoadUint64(&m.fbus)
+	f := m.fbus.Load()
-	ns := atomic.LoadUint64(&m.writeNanos)
+	ns := m.writeNanos.Load()
 
 	db := b - m.tickPrevB
 	dw := w - m.tickPrevW
@@ -115,9 +115,9 @@ func (m *metricsConn) flushTick(final bool) {
 	dns := ns - m.tickPrevNS
 	m.tickPrevB, m.tickPrevW, m.tickPrevF, m.tickPrevNS = b, w, f, ns
 
-	maxFBU := atomic.SwapUint64(&m.maxFBUBytes, 0)
+	maxFBU := m.maxFBUBytes.Swap(0)
-	maxRects := atomic.SwapUint64(&m.maxFBURects, 0)
+	maxRects := m.maxFBURects.Swap(0)
-	maxPkt := atomic.SwapUint64(&m.largestPkt, 0)
+	maxPkt := m.largestPkt.Swap(0)
 
 	period := time.Since(m.tickStart)
 	m.tickStart = time.Now()
@@ -144,7 +144,7 @@ func (m *metricsConn) flushTick(final bool) {
 // throttle JPEG quality or skip frames in response.
 func (m *metricsConn) BusyFraction() float64 {
 	now := time.Now()
-	ns := atomic.LoadUint64(&m.writeNanos)
+	ns := m.writeNanos.Load()
 
 	m.busyMu.Lock()
 	defer m.busyMu.Unlock()
@@ -179,30 +179,30 @@ func isFBUHeader(p []byte) bool {
 
 func (m *metricsConn) Write(p []byte) (int, error) {
 	if isFBUHeader(p) {
-		if b := atomic.SwapUint64(&m.fbuBytes, 0); b > 0 {
+		if b := m.fbuBytes.Swap(0); b > 0 {
-			if b > atomic.LoadUint64(&m.maxFBUBytes) {
+			if b > m.maxFBUBytes.Load() {
-				atomic.StoreUint64(&m.maxFBUBytes, b)
+				m.maxFBUBytes.Store(b)
 			}
 		}
-		if r := atomic.SwapUint64(&m.fbuRects, 0); r > 0 {
+		if r := m.fbuRects.Swap(0); r > 0 {
-			if r > atomic.LoadUint64(&m.maxFBURects) {
+			if r > m.maxFBURects.Load() {
-				atomic.StoreUint64(&m.maxFBURects, r)
+				m.maxFBURects.Store(r)
 			}
 		}
-		atomic.AddUint64(&m.fbus, 1)
+		m.fbus.Add(1)
 	}
 
 	t0 := time.Now()
 	n, err := m.Conn.Write(p)
-	atomic.AddUint64(&m.writeNanos, uint64(time.Since(t0).Nanoseconds()))
+	m.writeNanos.Add(uint64(time.Since(t0).Nanoseconds()))
-	atomic.AddUint64(&m.bytesOut, uint64(n))
+	m.bytesOut.Add(uint64(n))
-	atomic.AddUint64(&m.writes, 1)
+	m.writes.Add(1)
 	if !isFBUHeader(p) {
-		atomic.AddUint64(&m.fbuBytes, uint64(n))
+		m.fbuBytes.Add(uint64(n))
-		atomic.AddUint64(&m.fbuRects, 1)
+		m.fbuRects.Add(1)
 	}
-	if uint64(n) > atomic.LoadUint64(&m.largestPkt) {
+	if uint64(n) > m.largestPkt.Load() {
-		atomic.StoreUint64(&m.largestPkt, uint64(n))
+		m.largestPkt.Store(uint64(n))
 	}
 	return n, err
 }
@@ -213,11 +213,11 @@ func (m *metricsConn) Close() error {
 		if m.recorder == nil {
 			return
 		}
-		if b := atomic.SwapUint64(&m.fbuBytes, 0); b > atomic.LoadUint64(&m.maxFBUBytes) {
+		if b := m.fbuBytes.Swap(0); b > m.maxFBUBytes.Load() {
-			atomic.StoreUint64(&m.maxFBUBytes, b)
+			m.maxFBUBytes.Store(b)
 		}
-		if r := atomic.SwapUint64(&m.fbuRects, 0); r > atomic.LoadUint64(&m.maxFBURects) {
+		if r := m.fbuRects.Swap(0); r > m.maxFBURects.Load() {
-			atomic.StoreUint64(&m.maxFBURects, r)
+			m.maxFBURects.Store(r)
 		}
 		m.flushTick(true)
 	})

client/vnc/server/session.go

@@ -342,40 +342,7 @@ func (s *session) handleSetEncodings() error {
 		return err
 	}
 
-	var encs []string
+	encs, sendExtClipCaps, sendExtMouseAck := s.applyEncodings(buf, int(numEnc))
-	s.encMu.Lock()
-	// Per RFC 6143 §7.5.3 each SetEncodings replaces the previous list, so
-	// reset all flags before re-applying. extClipCapsSent stays sticky so
-	// we don't re-emit Caps every refresh.
-	s.resetEncodingCaps()
-	for i := range int(numEnc) {
-		enc := int32(binary.BigEndian.Uint32(buf[i*4 : i*4+4]))
-		if name := s.applyEncoding(enc); name != "" {
-			encs = append(encs, name)
-		}
-	}
-	if s.useTight && (s.tight == nil ||
-		s.tight.qualityLevel != s.clientJPEGQuality ||
-		s.tight.compressLevel != s.clientZlibLevel) {
-		// When we replace an in-use tightState the client's stream-0
-		// inflater carries dictionary state from the old deflater. Carry
-		// the pending-reset flag so the next Basic rect tells the client
-		// to reset its inflater before decoding.
-		replacing := s.tight != nil
-		s.tight = newTightStateWithLevels(s.clientJPEGQuality, s.clientZlibLevel)
-		if replacing {
-			s.tight.pendingZlibReset = true
-		}
-	}
-	sendExtClipCaps := s.clientSupportsExtClipboard && !s.extClipCapsSent
-	if sendExtClipCaps {
-		s.extClipCapsSent = true
-	}
-	sendExtMouseAck := s.clientSupportsExtMouseButtons && !s.extMouseAckSent
-	if sendExtMouseAck {
-		s.extMouseAckSent = true
-	}
-	s.encMu.Unlock()
 	if len(encs) > 0 {
 		s.log.Debugf("client supports encodings: %s", strings.Join(encs, ", "))
 	}
@@ -392,6 +359,57 @@ func (s *session) handleSetEncodings() error {
 	return nil
 }
 
+// applyEncodings parses the SetEncodings body, updates capability flags,
+// rebuilds the tight state if quality/level changed, and reports which
+// one-shot acknowledgements still need to be sent.
+func (s *session) applyEncodings(buf []byte, numEnc int) (names []string, sendExtClipCaps, sendExtMouseAck bool) {
+	s.encMu.Lock()
+	defer s.encMu.Unlock()
+	// Per RFC 6143 §7.5.3 each SetEncodings replaces the previous list, so
+	// reset all flags before re-applying. extClipCapsSent stays sticky so
+	// we don't re-emit Caps every refresh.
+	s.resetEncodingCaps()
+	for i := range numEnc {
+		enc := int32(binary.BigEndian.Uint32(buf[i*4 : i*4+4]))
+		if name := s.applyEncoding(enc); name != "" {
+			names = append(names, name)
+		}
+	}
+	s.refreshTightStateLocked()
+	sendExtClipCaps = s.clientSupportsExtClipboard && !s.extClipCapsSent
+	if sendExtClipCaps {
+		s.extClipCapsSent = true
+	}
+	sendExtMouseAck = s.clientSupportsExtMouseButtons && !s.extMouseAckSent
+	if sendExtMouseAck {
+		s.extMouseAckSent = true
+	}
+	return names, sendExtClipCaps, sendExtMouseAck
+}
+
+// refreshTightStateLocked reallocates s.tight when the requested quality
+// or compression level no longer matches the cached state. Caller holds
+// s.encMu.
+func (s *session) refreshTightStateLocked() {
+	if !s.useTight {
+		return
+	}
+	if s.tight != nil &&
+		s.tight.qualityLevel == s.clientJPEGQuality &&
+		s.tight.compressLevel == s.clientZlibLevel {
+		return
+	}
+	// When we replace an in-use tightState the client's stream-0
+	// inflater carries dictionary state from the old deflater. Carry
+	// the pending-reset flag so the next Basic rect tells the client
+	// to reset its inflater before decoding.
+	replacing := s.tight != nil
+	s.tight = newTightStateWithLevels(s.clientJPEGQuality, s.clientZlibLevel)
+	if replacing {
+		s.tight.pendingZlibReset = true
+	}
+}
+
 // resetEncodingCaps zeroes the encoding capability flags so the next pass
 // through applyEncoding reflects exactly what the client just advertised.
 // Caller holds s.encMu. tight / copyRectDet allocations are kept; their