ronantakizawa/codereview-bench huggingface dataset:
Repository: etcd-io/etcd
File: client/pkg/verify/verify.go (Go)

Code before:
		os.Setenv(envVerify, previousEnv)
	}
}

// EnableAllVerifications enables verification and returns a function
// that can be used to bring the original settings.
func EnableAllVerifications() func() {
	return EnableVerifications(envVerifyValueAll)
}

// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		// DO NOT CHANGE; the line below is replaced with Antithesis assertions when validating etcd on their platform.
		// antithesis_verifier_placeholder
		verifier(ok, msg, details)
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	if condition {
		panic(fmt.Sprintf("%s. details: %v.", msg, details))
	}
}


Review comment (suggestion):
```suggestion
		// DO NOT CHANGE LINES BELOW; they are replaced with Antithesis assertions when validating etcd on their platform
```

Code after:
}

// EnableAllVerifications enables verification and returns a function
// that can be used to bring the original settings.
func EnableAllVerifications() func() {
	return EnableVerifications(envVerifyValueAll)
}

// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		verifier(ok, msg, details)
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	if !condition {
		panic(fmt.Sprintf("%s. details: %v.", msg, details))
	}
}

// Assert will panic with a given formatted message if the given condition is false.
func Assert(condition bool, msg string, v ...any) {
	if !condition {
		panic(fmt.Sprintf("assertion failed: "+msg, v...))
	}
}
Repository: etcd-io/etcd
File: client/pkg/verify/verify.go (Go)

Code before:
	os.Setenv(envVerify, string(verification))
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// EnableAllVerifications enables verification and returns a function
// that can be used to bring the original settings.
func EnableAllVerifications() func() {
	return EnableVerifications(envVerifyValueAll)
}

// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		// DO NOT CHANGE; the line below is replaced with Antithesis assertions when validating etcd on their platform.
		// antithesis_verifier_placeholder
		verifier(ok, msg, details)
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	if condition {
		panic(fmt.Sprintf("%s. details: %v.", msg, details))
	}
}


Review comment (suggestion):
```suggestion
		// antithesis_verifier_placeholder_start
		verifier(ok, msg, details)
		// antithesis_verifier_placeholder_end
```
Maybe wrap the replaced lines to make it clear.

Code after:
		os.Setenv(envVerify, previousEnv)
	}
}

// EnableAllVerifications enables verification and returns a function
// that can be used to bring the original settings.
func EnableAllVerifications() func() {
	return EnableVerifications(envVerifyValueAll)
}

// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		verifier(ok, msg, details)
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	if !condition {
		panic(fmt.Sprintf("%s. details: %v.", msg, details))
	}
}

// Assert will panic with a given formatted message if the given condition is false.
func Assert(condition bool, msg string, v ...any) {
	if !condition {
		panic(fmt.Sprintf("assertion failed: "+msg, v...))
	}
}
Repository: etcd-io/etcd
File: client/pkg/verify/verify.go (Go)

Code before:
		os.Setenv(envVerify, previousEnv)
	}
}

// EnableAllVerifications enables verification and returns a function
// that can be used to bring the original settings.
func EnableAllVerifications() func() {
	return EnableVerifications(envVerifyValueAll)
}

// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		// DO NOT CHANGE LINE BELOW; it is replaced with Antithesis assertions when validating etcd on their platform.
		// antithesis_verifier_placeholder_start
		verifier(ok, msg, details)
		// antithesis_verifier_placeholder_end
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	if condition {
		panic(fmt.Sprintf("%s. details: %v.", msg, details))
	}
}


Review comment (suggestion):
```suggestion
		// DO NOT CHANGE LINES BELOW; it is replaced with Antithesis assertions when validating etcd on their platform.
```

Code after:
}

// EnableAllVerifications enables verification and returns a function
// that can be used to bring the original settings.
func EnableAllVerifications() func() {
	return EnableVerifications(envVerifyValueAll)
}

// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		verifier(ok, msg, details)
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	if !condition {
		panic(fmt.Sprintf("%s. details: %v.", msg, details))
	}
}

// Assert will panic with a given formatted message if the given condition is false.
func Assert(condition bool, msg string, v ...any) {
	if !condition {
		panic(fmt.Sprintf("assertion failed: "+msg, v...))
	}
}
Repository: etcd-io/etcd
File: pkg/traceutil/trace.go (Go)

Code before:
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// Package traceutil implements tracing utilities using "context".
package traceutil

import (
	"context"
	"fmt"
	"math/rand"
	"strings"
	"time"

	"go.uber.org/zap"
)

// TraceKey is used as a key of context for Trace.
type TraceKey struct{}

// StartTimeKey is used as a key of context for start time of operation.
type StartTimeKey struct{}

// Field is a kv pair to record additional details of the trace.
type Field struct {
	Key   string
	Value any
}

func (f *Field) format() string {
	return fmt.Sprintf("%s:%v; ", f.Key, f.Value)
}

func writeFields(fields []Field) string {
	if len(fields) == 0 {
		return ""
	}
	var buf strings.Builder
	buf.WriteString("{")
	for _, f := range fields {
		buf.WriteString(f.format())
	}
	buf.WriteString("}")
	return buf.String()
}

type Trace struct {
	operation    string
	lg           *zap.Logger
	fields       []Field

Review comment (suggestion):
maybe we should introduce syncOnce for this tracer. just a case that it could be race-condition. it's not blocker

Code after:
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// Package traceutil implements tracing utilities using "context".
package traceutil

import (
	"context"
	"fmt"
	"math/rand"
	"strings"
	"time"

	"go.opentelemetry.io/otel/trace"
	"go.opentelemetry.io/otel/trace/noop"
	"go.uber.org/zap"
)

const instrumentationScope = "go.etcd.io/etcd"

var Tracer trace.Tracer = noop.NewTracerProvider().Tracer(instrumentationScope)

func Init(tp trace.TracerProvider) {
	Tracer = tp.Tracer(instrumentationScope)
}

// TraceKey is used as a key of context for Trace.
type TraceKey struct{}

// StartTimeKey is used as a key of context for start time of operation.
type StartTimeKey struct{}

// Field is a kv pair to record additional details of the trace.
type Field struct {
	Key   string
	Value any
}

func (f *Field) format() string {
	return fmt.Sprintf("%s:%v; ", f.Key, f.Value)
}

func writeFields(fields []Field) string {
	if len(fields) == 0 {
		return ""
	}
	var buf strings.Builder
	buf.WriteString("{")
	for _, f := range fields {
Repository: etcd-io/etcd
File: tests/integration/cache_test.go (Go)

Code before:
		Kv: &mvccpb.KeyValue{
			Key:         []byte("/foo/future"),
			ModRevision: 12,
		},
	}
	rev12PutFooTx2 := &clientv3.Event{
		Type: clientv3.EventTypePut,
		Kv: &mvccpb.KeyValue{
			Key:            []byte("/foo/tx2"),
			Value:          []byte("b"),
			CreateRevision: 12,
			ModRevision:    12,
			Version:        1,
		},
	}

	tcs := []struct {
		name       string
		key        string
		opts       []clientv3.OpOption
		wantEvents []*clientv3.Event
	}{
		{
			name:       "Watch single key existing /foo/c",
			key:        "/foo/c",
			opts:       nil,
			wantEvents: []*clientv3.Event{rev6PutFooC},
		},
		{
			name:       "Watch single key non‑existent /doesnotexist",
			key:        "/doesnotexist",
			opts:       nil,
			wantEvents: nil,
		},
		{
			name:       "Watch range empty",
			key:        "",
			opts:       []clientv3.OpOption{clientv3.WithRange("")},
			wantEvents: nil,
		},
		{
			name:       "Watch range [/foo/a, /foo/b)",
			key:        "/foo/a",
			opts:       []clientv3.OpOption{clientv3.WithRange("/foo/b")},
			wantEvents: []*clientv3.Event{rev2PutFooA, rev4DeleteFooA, rev5PutFooA},
		},
		{
			name:       "Watch with prefix /foo/b",
			key:        "/foo/b",
			opts:       []clientv3.OpOption{clientv3.WithPrefix()},
			wantEvents: []*clientv3.Event{rev3PutFooB, rev5DeleteFooB, rev7PutFooBar, rev8PutFooBaz},

Review comment (suggestion):
```suggestion
			opts:       []clientv3.OpOption{clientv3.WithRev(2)},,
```


Code after:
		Kv: &mvccpb.KeyValue{
			Key:         []byte("/foo/future"),
			ModRevision: 12,
		},
	}
	rev12PutFooTx2 := &clientv3.Event{
		Type: clientv3.EventTypePut,
		Kv: &mvccpb.KeyValue{
			Key:            []byte("/foo/tx2"),
			Value:          []byte("b"),
			CreateRevision: 12,
			ModRevision:    12,
			Version:        1,
		},
	}

	tcs := []struct {
		name       string
		key        string
		opts       []clientv3.OpOption
		wantEvents []*clientv3.Event
	}{
		{
			name:       "Watch single key existing /foo/c",
			key:        "/foo/c",
			opts:       []clientv3.OpOption{clientv3.WithRev(2)},
			wantEvents: []*clientv3.Event{rev6PutFooC},
		},
		{
			name:       "Watch single key non‑existent /doesnotexist",
			key:        "/doesnotexist",
			opts:       []clientv3.OpOption{clientv3.WithRev(2)},
			wantEvents: nil,
		},
		{
			name:       "Watch range empty",
			key:        "",
			opts:       []clientv3.OpOption{clientv3.WithRange(""), clientv3.WithRev(2)},
			wantEvents: nil,
		},
		{
			name:       "Watch range [/foo/a, /foo/b)",
			key:        "/foo/a",
			opts:       []clientv3.OpOption{clientv3.WithRange("/foo/b"), clientv3.WithRev(2)},
			wantEvents: []*clientv3.Event{rev2PutFooA, rev4DeleteFooA, rev5PutFooA},
		},
		{
			name:       "Watch with prefix /foo/b",
			key:        "/foo/b",
			opts:       []clientv3.OpOption{clientv3.WithPrefix(), clientv3.WithRev(2)},
			wantEvents: []*clientv3.Event{rev3PutFooB, rev5DeleteFooB, rev7PutFooBar, rev8PutFooBaz},
Repository: etcd-io/etcd
File: cache/cache.go (Go)

Code before:
	cache *Cache,
	demux *demux,
	onFirstResponse func(),
) error {
	for resp := range watchChan {
		onFirstResponse()

		if err := resp.Err(); err != nil {
			if errors.Is(err, rpctypes.ErrCompacted) {
				select {
				case <-cache.ready:
					// TODO: Reinitialize cache.ready safely; current direct channel assignment can race with concurrent watchers
					cache.ready = make(chan struct{})
				default:
				}
				demux.Purge()
			}
			return err
		}
		demux.Broadcast(resp.Events)
	}
	return nil
}

func (c *Cache) validateWatch(key string, opts ...clientv3.OpOption) (pred KeyPredicate, err error) {
	op := clientv3.OpWatch(key, opts...)

	if op.IsPrevKV() ||
		op.IsFragment() ||
		op.IsProgressNotify() ||
		op.IsCreatedNotify() ||
		op.IsFilterPut() ||
		op.IsFilterDelete() {
		return nil, ErrUnsupportedWatch
	}

	startKey := []byte(key)
	endKey := op.RangeBytes() // nil = single key, {0}=FromKey, else explicit range

	if err := c.validateWatchRange(startKey, endKey); err != nil {
		return nil, err
	}
	return KeyPredForRange(startKey, endKey), nil
}

func (c *Cache) validateWatchRange(startKey, endKey []byte) error {
	prefixStart := []byte(c.prefix)
	prefixEnd := []byte(clientv3.GetPrefixRangeEnd(c.prefix))

	isSingleKey := len(endKey) == 0
	isFromKey := len(endKey) == 1 && endKey[0] == 0

Review comment (suggestion):
nit; we don't need to call OpWatch here again. 
Let's pass in `op` directly since we call it in the parent function anyway 

Code after:
	cache *Cache,
	demux *demux,
	onFirstResponse func(),
) error {
	for resp := range watchChan {
		onFirstResponse()

		if err := resp.Err(); err != nil {
			if errors.Is(err, rpctypes.ErrCompacted) {
				select {
				case <-cache.ready:
					// TODO: Reinitialize cache.ready safely; current direct channel assignment can race with concurrent watchers
					cache.ready = make(chan struct{})
				default:
				}
				demux.Purge()
			}
			return err
		}
		demux.Broadcast(resp.Events)
	}
	return nil
}

func (c *Cache) validateWatch(key string, op clientv3.Op) (pred KeyPredicate, err error) {
	if op.IsPrevKV() ||
		op.IsFragment() ||
		op.IsProgressNotify() ||
		op.IsCreatedNotify() ||
		op.IsFilterPut() ||
		op.IsFilterDelete() {
		return nil, ErrUnsupportedWatch
	}

	startKey := []byte(key)
	endKey := op.RangeBytes() // nil = single key, {0}=FromKey, else explicit range

	if err := c.validateWatchRange(startKey, endKey); err != nil {
		return nil, err
	}
	return KeyPredForRange(startKey, endKey), nil
}

func (c *Cache) validateWatchRange(startKey, endKey []byte) error {
	prefixStart := []byte(c.prefix)
	prefixEnd := []byte(clientv3.GetPrefixRangeEnd(c.prefix))

	isSingleKey := len(endKey) == 0
	isFromKey := len(endKey) == 1 && endKey[0] == 0

	switch {
Repository: etcd-io/etcd
File: pkg/netutil/host_normalize.go (Go)

Code before:
// Copyright 2015 The etcd Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package netutil

import (
	"net"
	"net/url"
	"strings"
)

// urlsHostNormalizedEqual compares two URLs for scheme, normalized host (including IPv6), and path equality.
func urlsHostNormalizedEqual(a, b url.URL) bool {
	return a.Scheme == b.Scheme &&
		normalizeHost(a.Host) == normalizeHost(b.Host) &&

Review comment (security):
```suggestion
// Copyright 2025 The etcd Authors
```

Code after:
// Copyright 2025 The etcd Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package netutil

import (
	"net"
	"net/url"
	"strings"
)

// urlsHostNormalizedEqual compares two URLs for scheme, normalized host (including IPv6), and path equality.
func urlsHostNormalizedEqual(a, b url.URL) bool {
	return a.Scheme == b.Scheme &&
		normalizeHost(a.Host) == normalizeHost(b.Host) &&
Repository: etcd-io/etcd
File: pkg/netutil/host_normalize_test.go (Go)

Code before:
// Copyright 2015 The etcd Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package netutil

import (
	"context"
	"testing"

	"github.com/stretchr/testify/assert"
	"go.uber.org/zap/zaptest"
)

func TestIPv6AddressNormalization(t *testing.T) {
	testCases := []struct {

Review comment (security):
```suggestion
// Copyright 2025 The etcd Authors
```

Code after:
// Copyright 2025 The etcd Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package netutil

import (
	"testing"

	"github.com/stretchr/testify/assert"
	"go.uber.org/zap/zaptest"
)

func TestIPv6AddressNormalization(t *testing.T) {
	testCases := []struct {
		name     string
Repository: etcd-io/etcd
File: pkg/netutil/host_normalize.go (Go)

Code before:
import (
	"net"
	"net/url"
	"strings"
)

// urlsHostNormalizedEqual compares two URLs for scheme, normalized host (including IPv6), and path equality.
func urlsHostNormalizedEqual(a, b url.URL) bool {
	return a.Scheme == b.Scheme &&
		normalizeHost(a.Host) == normalizeHost(b.Host) &&
		a.Path == b.Path
}

// normalizeHost returns the canonical string for the host and normalizes IPv6 and IPv4 addresses.
func normalizeHost(host string) string {
	hostOnly, port, err := net.SplitHostPort(host)
	if err != nil {
		hostOnly = host
		port = ""
	}

	// Check if hostOnly is an IPv6 address. It could be with or without brackets.
	ipStr := hostOnly
	if strings.HasPrefix(hostOnly, "[") && strings.HasSuffix(hostOnly, "]") {
		ipStr = hostOnly[1 : len(hostOnly)-1]
	}

	if ip := net.ParseIP(ipStr); ip != nil {
		if ip.To4() == nil {
			// For IPv6 address, always use brackets when there is a port.
			if port != "" {
				return "[" + ip.String() + "]" + normalizePort(port)
			}
			return "[" + ip.String() + "]"
		}
		// IPv4 address
		return ip.String() + normalizePort(port)
	}
	return host
}

func normalizePort(port string) string {
	if port == "" {
		return ""
	}
	return ":" + port
}


Review comment (suggestion):
```suggestion
	ipStr := strings.Trim(hostOnly, "[]")
```

Code after:
import (
	"net"
	"net/url"
	"strings"
)

// urlsHostNormalizedEqual compares two URLs for scheme, normalized host (including IPv6), and path equality.
func urlsHostNormalizedEqual(a, b url.URL) bool {
	return a.Scheme == b.Scheme &&
		normalizeHost(a.Host) == normalizeHost(b.Host) &&
		a.Path == b.Path
}

// normalizeHost returns the canonical string for the host and normalizes IPv6 and IPv4 addresses.
func normalizeHost(host string) string {
	hostOnly, port, err := net.SplitHostPort(host)
	if err != nil {
		hostOnly = host
		port = ""
	}

	// Check if hostOnly is an IPv6 address. It could be with or without brackets.
	ipStr := strings.Trim(hostOnly, "[]")
	if ip := net.ParseIP(ipStr); ip != nil {
		if ip.To4() == nil {
			// For IPv6 address, always use brackets when there is a port.
			return "[" + ip.String() + "]" + normalizePort(port)
		}
		// IPv4 address
		return ip.String() + normalizePort(port)
	}
	return host
}

func normalizePort(port string) string {
	if port == "" {
		return ""
	}
	return ":" + port
}