ronantakizawa/codereview-bench huggingface dataset:
Repository: netty/netty
File: resolver-dns/src/main/java/io/netty/resolver/dns/ResolvConf.java (Java)

Code before:
/*
 * Copyright 2018 The Netty Project
 *
 * The Netty Project licenses this file to you 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:
 *
 *   https://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 io.netty.resolver.dns;

import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

/**
 * Looks up the {@code nameserver}s from the {@code /etc/resolv.conf} file, intended for Linux and macOS.

Review comment (suggestion):

```suggestion
 * Copyright 2024 The Netty Project
```

Code after:
/*
 * Copyright 2024 The Netty Project
 *
 * The Netty Project licenses this file to you 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:
 *
 *   https://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 io.netty.resolver.dns;

import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

/**
 * Looks up the {@code nameserver}s from the {@code /etc/resolv.conf} file, intended for Linux and macOS.
Repository: netty/netty
File: resolver-dns/src/test/java/io/netty/resolver/dns/ResolvConfTest.java (Java)

Code before:
/*
 * Copyright 2018 The Netty Project
 *
 * The Netty Project licenses this file to you 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:
 *
 *   https://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 io.netty.resolver.dns;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.OS;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.io.BufferedReader;
import java.io.StringReader;
import java.net.InetSocketAddress;

Review comment (suggestion):
```suggestion
 * Copyright 2024 The Netty Project
```

Code after:
/*
 * Copyright 2024 The Netty Project
 *
 * The Netty Project licenses this file to you 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:
 *
 *   https://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 io.netty.resolver.dns;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.OS;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.io.BufferedReader;
import java.io.StringReader;
import java.net.InetSocketAddress;
Repository: netty/netty
File: resolver-dns/src/main/java/io/netty/resolver/dns/ResolvConf.java (Java)

Code before:
        }
        throw new IllegalStateException("/etc/resolv.conf could not be read");
    }

    private ResolvConf(BufferedReader reader) throws IOException {
        List<InetSocketAddress> nameservers = new ArrayList<InetSocketAddress>();
        String ln;
        while ((ln = reader.readLine()) != null) {
            ln = ln.trim();
            if (ln.isEmpty()) {
                continue;
            }

            if (ln.startsWith("nameserver")) {
                ln = ln.substring("nameserver".length()).trim();
                nameservers.add(new InetSocketAddress(ln, 53));
            }
        }
        this.nameservers = Collections.unmodifiableList(nameservers);
    }

    List<InetSocketAddress> getNameservers() {
        return nameservers;
    }

    static final class ResolvConfLazy {
        static final ResolvConf machineResolvConf;

        static {
            ResolvConf resolvConf;
            try {
                resolvConf = ResolvConf.fromFile("/etc/resolv.conf");
            } catch (IOException e) {
                resolvConf = null;
            }
            machineResolvConf = resolvConf;
        }
    }
}


Review comment (suggestion):
```suggestion
    private static final class ResolvConfLazy {
```

Code after:
        ResolvConf resolvConv = ResolvConfLazy.machineResolvConf;
        if (resolvConv != null) {
            return resolvConv;
        }
        throw new IllegalStateException("/etc/resolv.conf could not be read");
    }

    private ResolvConf(BufferedReader reader) throws IOException {
        List<InetSocketAddress> nameservers = new ArrayList<InetSocketAddress>();
        String ln;
        while ((ln = reader.readLine()) != null) {
            ln = ln.trim();
            if (ln.isEmpty()) {
                continue;
            }

            if (ln.startsWith("nameserver")) {
                ln = ln.substring("nameserver".length()).trim();
                nameservers.add(new InetSocketAddress(ln, 53));
            }
        }
        this.nameservers = Collections.unmodifiableList(nameservers);
    }

    List<InetSocketAddress> getNameservers() {
        return nameservers;
    }

    private static final class ResolvConfLazy {
        static final ResolvConf machineResolvConf;

        static {
            ResolvConf resolvConf;
            try {
                resolvConf = ResolvConf.fromFile("/etc/resolv.conf");
            } catch (IOException e) {
                resolvConf = null;
            }
            machineResolvConf = resolvConf;
        }
    }
}
Repository: netty/netty
File: codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttTopicSubscription.java (Java)

Code before:
        this.topicFilter = topicFilter;
        this.option = option;
    }

    /**
     * @deprecated use topicFilter
     */
    @Deprecated
    public String topicName() {
        return topicFilter;
    }

    public String topicFilter() {
        return topicFilter;
    }

    /**
     * Rewrite topic filter.
     * <p>
     * 
     * Many IoT devices do not support reconfiguration or upgrade, so it is hard to
     * change their subscribed topics. To resolve this issue, MQTT server may offer
     * topic rewrite capability.
     * 
     * @param topicFilter Topic to rewrite to
     */
    public void setTopicFilter(String topicFilter) {
        this.topicFilter = topicFilter;
    }

    public MqttQoS qualityOfService() {
        return option.qos();
    }

    public MqttSubscriptionOption option() {
        return option;
    }

    @Override
    public String toString() {
        return new StringBuilder(StringUtil.simpleClassName(this))
                .append('[')
                .append("topicFilter=").append(topicFilter)
                .append(", option=").append(this.option)
                .append(']')
                .toString();
    }
}


Review comment (suggestion):
```suggestion
    /**
     * Rewrite topic filter.
     * <p>
     *
     * Many IoT devices do not support reconfiguration or upgrade, so it is hard to
     * change their subscribed topics. To resolve this issue, MQTT server may offer
     * topic rewrite capability.
     *
     * @param topicFilter Topic to rewrite to
     */
```

Code after:
        this.topicFilter = topicFilter;
        this.option = option;
    }

    /**
     * @deprecated use topicFilter
     */
    @Deprecated
    public String topicName() {
        return topicFilter;
    }

    public String topicFilter() {
        return topicFilter;
    }

    /**
     * Rewrite topic filter.
     * <p>
     *
     * Many IoT devices do not support reconfiguration or upgrade, so it is hard to
     * change their subscribed topics. To resolve this issue, MQTT server may offer
     * topic rewrite capability.
     *
     * @param topicFilter Topic to rewrite to
     */
    public void setTopicFilter(String topicFilter) {
        this.topicFilter = topicFilter;
    }

    public MqttQoS qualityOfService() {
        return option.qos();
    }

    public MqttSubscriptionOption option() {
        return option;
    }

    @Override
    public String toString() {
        return new StringBuilder(StringUtil.simpleClassName(this))
                .append('[')
                .append("topicFilter=").append(topicFilter)
                .append(", option=").append(this.option)
                .append(']')
                .toString();
    }
}
Repository: netty/netty
File: transport/src/main/java/io/netty/channel/DefaultChannelId.java (Java)

Code before:

            for (int k = 0; k < len; k++) {
                byte x = data[k];
                byte y = otherData[k];
                if (x != y) {
                    // treat these as unsigned bytes for comparison
                    return (x & 0xff) - (y & 0xff);
                }
            }
            return len1 - len2;
        }

        return asLongText().compareTo(o.asLongText());
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj) {
            return true;
        }
        if (!(obj instanceof DefaultChannelId)) {
            return false;
        }
        DefaultChannelId other = (DefaultChannelId) obj;
        return hashCode == other.hashCode && Arrays.equals(data, other.data);
    }

    @Override
    public String toString() {
        return asShortText();
    }
}


Review comment (suggestion):
I think this is over-thinking it. As long as the sort order is consistent, it should be fine. No need to make it match the long-text sorting, IMO.

Code after:
            return 0;
        }
        if (o instanceof DefaultChannelId) {
            // lexicographic comparison
            final byte[] otherData = ((DefaultChannelId) o).data;
            int len1 = data.length;
            int len2 = otherData.length;
            int len = Math.min(len1, len2);

            for (int k = 0; k < len; k++) {
                byte x = data[k];
                byte y = otherData[k];
                if (x != y) {
                    // treat these as unsigned bytes for comparison
                    return (x & 0xff) - (y & 0xff);
                }
            }
            return len1 - len2;
        }

        return asLongText().compareTo(o.asLongText());
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj) {
            return true;
        }
        if (!(obj instanceof DefaultChannelId)) {
            return false;
        }
        DefaultChannelId other = (DefaultChannelId) obj;
        return hashCode == other.hashCode && Arrays.equals(data, other.data);
    }

    @Override
    public String toString() {
        return asShortText();
    }
}
Repository: netty/netty
File: buffer/src/test/java/io/netty/buffer/ByteBufStreamTest.java (Java)

Code before:
        assertEquals(0, i);
    }

    @Test
    public void testGeneralByteBufOutputStream() throws Exception {
        // case1
        ByteBuf buf = PooledByteBufAllocator.DEFAULT.buffer(16);
        buf.writeBytes(new byte[] { 1, 2, 3, 4, 5, 6 });
        final ByteBufOutputStream out = new ByteBufOutputStream(buf, false);
        try {
            out.writeBoolean(true);
            out.writeBoolean(false);
            out.writeByte(42);
            out.writeByte(224);
            out.writeBytes("Hello, World!");
            out.write(new byte[]{1, 3, 3, 4}, 0, 0);
        } finally {
            out.close();
        }
        int i = ReferenceCountUtil.refCnt(out.buffer());
        assertEquals(1, i);

        // When releaseOnClose is not set or releaseOnClose is false, ByteBuf must be released manually.
        out.buffer().release();
        i = ReferenceCountUtil.refCnt(out.buffer());
        assertEquals(0, i);
    }
}


Review comment (suggestion):
```suggestion
        assertEquals(1, out.buffer().refCnt());

        // When releaseOnClose is not set or releaseOnClose is false, ByteBuf must be released manually.
        out.buffer().release();
        assertEquals(0, out.buffer().refCnt());
```

Code after:
    }

    @Test
    public void testGeneralByteBufOutputStream() throws Exception {
        // case1
        ByteBuf buf = PooledByteBufAllocator.DEFAULT.buffer(16);
        buf.writeBytes(new byte[] { 1, 2, 3, 4, 5, 6 });
        final ByteBufOutputStream out = new ByteBufOutputStream(buf, false);
        try {
            out.writeBoolean(true);
            out.writeBoolean(false);
            out.writeByte(42);
            out.writeByte(224);
            out.writeBytes("Hello, World!");
            out.write(new byte[]{1, 3, 3, 4}, 0, 0);
        } finally {
            out.close();
        }
        assertEquals(1, out.buffer().refCnt());

        // When releaseOnClose is not set or releaseOnClose is false, ByteBuf must be released manually.
        out.buffer().release();
        assertEquals(0, out.buffer().refCnt());
    }
}
Repository: netty/netty
File: handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java (Java)

Code before:
        @Override
        public long nextLong() {
            count++;
            return super.nextLong();
        }

        @Override
        public boolean nextBoolean() {
            count++;
            return super.nextBoolean();
        }

        @Override
        public float nextFloat() {
            count++;
            return super.nextFloat();
        }

        @Override
        public double nextDouble() {
            count++;
            return super.nextDouble();
        }

        @Override
        public synchronized double nextGaussian() {
            count++;
            return super.nextGaussian();
        }

        public int getCount() {
            return count;
        }
    }
}


Review comment (suggestion):
```suggestion
        public double nextGaussian() {
```

Code after:
        SSLEngine server_engine = server_context.newEngine(UnpooledByteBufAllocator.DEFAULT);
        assertFalse(server_engine.getWantClientAuth());
        assertTrue(server_engine.getNeedClientAuth());
        server_engine.closeInbound();
        server_engine.closeOutbound();
    }

    private static final class SpySecureRandom extends SecureRandom {
        private int count;

        @Override
        public int nextInt() {
            count++;
            return super.nextInt();
        }

        @Override
        public int nextInt(int bound) {
            count++;
            return super.nextInt(bound);
        }

        @Override
        public long nextLong() {
            count++;
            return super.nextLong();
        }

        @Override
        public boolean nextBoolean() {
            count++;
            return super.nextBoolean();
        }

        @Override
        public float nextFloat() {
            count++;
            return super.nextFloat();
        }

        @Override
        public double nextDouble() {
            count++;
            return super.nextDouble();
        }

        @Override
        public double nextGaussian() {
            count++;
            return super.nextGaussian();
        }
Repository: netty/netty
File: buffer/src/test/java/io/netty/buffer/ReadOnlyDirectByteBufferBufTest.java (Java)

Code before:
            }
            file.delete();
        }
    }

    @Test
    public void testMemoryAddress() {
        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
        try {
            assertFalse(buf.hasMemoryAddress());
            try {
                buf.memoryAddress();
                fail();
            } catch (UnsupportedOperationException expected) {
                // expected
            }
        } finally {
            buf.release();
        }
    }

    @Test
    public void testDuplicate() {
        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
        try {
            assertEquals(buf.getClass(), buf.duplicate().getClass());
        } finally {
            buf.release();
        }
    }

    @Test
    public void testSlice() {
        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
        try {
            assertEquals(buf.getClass(), buf.slice().getClass());
        } finally {
            buf.release();
        }
    }

    enum WritableParam {
        None,
        Duplicate,
        Slice
    }

    @ParameterizedTest
    @EnumSource(WritableParam.class)
    void testIsWritable(WritableParam param) {
        ByteBuffer buffer = allocate(24);

Review comment (suggestion):
I don't think we should assert on the concrete classes, but rather that the resulting buffers are read-only and contain the expected contents.

Code after:
            if (b2 != null) {
                b2.release();
            }
            if (output != null) {
                output.close();
            }
            if (input != null) {
                input.close();
            }
            file.delete();
        }
    }

    @Test
    public void testMemoryAddress() {
        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
        try {
            assertFalse(buf.hasMemoryAddress());
            try {
                buf.memoryAddress();
                fail();
            } catch (UnsupportedOperationException expected) {
                // expected
            }
        } finally {
            buf.release();
        }
    }

    @Test
    public void testDuplicate() {
        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
        try {
            assertTrue(buf.duplicate().isReadOnly());
        } finally {
            buf.release();
        }
    }

    @Test
    public void testSlice() {
        ByteBuf buf = buffer(allocate(8).asReadOnlyBuffer());
        try {
            assertTrue(buf.slice().isReadOnly());
        } finally {
            buf.release();
        }
    }

    enum WritableParam {
        None,
Repository: netty/netty
File: buffer/src/main/java/io/netty/buffer/PoolThreadCache.java (Java)

Code before:
            Entry entry = RECYCLER.get();
            entry.chunk = chunk;
            entry.nioBuffer = nioBuffer;
            entry.handle = handle;
            entry.normCapacity = normCapacity;
            return entry;
        }

        @SuppressWarnings("rawtypes")
        private static final ObjectPool<Entry> RECYCLER = ObjectPool.newPool(new ObjectCreator<Entry>() {
            @SuppressWarnings("unchecked")
            @Override
            public Entry newObject(Handle<Entry> handle) {
                return new Entry(handle);
            }
        });
    }

    private static final class FreeOnFinalize {
        private final PoolThreadCache cache;

        private FreeOnFinalize(PoolThreadCache cache) {
            this.cache = cache;
        }

        /// TODO: In the future when we move to Java9+ we should use java.lang.ref.Cleaner.
        @SuppressWarnings({"FinalizeDeclaration", "deprecation"})
        @Override
        protected void finalize() throws Throwable {
            try {
                super.finalize();
            } finally {
                cache.free(true);
            }
        }
    }
}


Review comment (suggestion):
Hmm... if there can be a race I think we need to use an `AtomicReference` and `compareAndSet()`

Code after:
            @Override
            public Entry newObject(Handle<Entry> handle) {
                return new Entry(handle);
            }
        });
    }

    private static final class FreeOnFinalize {

        private volatile PoolThreadCache cache;

        private FreeOnFinalize(PoolThreadCache cache) {
            this.cache = cache;
        }

        /// TODO: In the future when we move to Java9+ we should use java.lang.ref.Cleaner.
        @SuppressWarnings({"FinalizeDeclaration", "deprecation"})
        @Override
        protected void finalize() throws Throwable {
            try {
                super.finalize();
            } finally {
                PoolThreadCache cache = this.cache;
                // this can race with a non-finalizer thread calling free: regardless who wins, the cache will be
                // null out
                this.cache = null;
                if (cache != null) {
                    cache.free(true);
                }
            }
        }
    }
}