ronantakizawa/codereview-bench huggingface dataset:
Repository: netty/netty
File: handler/src/main/java/io/netty/handler/ssl/util/SelfSignedCertificate.java (Java)

Code before:
        SecureRandom random;
        int bits = DEFAULT_KEY_LENGTH_BITS;
        Date notBefore = DEFAULT_NOT_BEFORE;
        Date notAfter = DEFAULT_NOT_AFTER;
        String algorithm = "RSA";

        // fields that are populated on demand
        Throwable failure;
        KeyPair keypair;
        PrivateKey privateKey;
        String[] paths;

        private Builder() {
        }

        /**
         * Set the fully-qualified domain name of the certificate that should be generated.
         *
         * @param fqdn The FQDN
         * @return This builder
         */
        public Builder fqdn(String fqdn) {
            ObjectUtil.checkNotNullWithIAE(fqdn, "fqdn");
            this.fqdn = fqdn;
            return this;
        }

        /**
         * Set the RNG to use for key generation. This setting is not supported by the keytool-based generator.
         *
         * @param random The CSPRNG
         * @return This builder
         */
        public Builder random(SecureRandom random) {
            this.random = random;
            return this;
        }

        /**
         * Set the key size.
         *
         * @param bits The key size
         * @return This builder
         */
        public Builder bits(int bits) {
            this.bits = bits;
            return this;
        }

        /**
         * Set the start of the certificate validity period.

Review comment (suggestion):
```suggestion
        public Builder fqdn(String fqdn) {
            this.fqdn = ObjectUtil.checkNotNullWithIAE(fqdn, "fqdn");
            return this;
        }
```

Code after:
        SecureRandom random;
        int bits = DEFAULT_KEY_LENGTH_BITS;
        Date notBefore = DEFAULT_NOT_BEFORE;
        Date notAfter = DEFAULT_NOT_AFTER;
        String algorithm = "RSA";

        // fields that are populated on demand
        Throwable failure;
        KeyPair keypair;
        PrivateKey privateKey;
        String[] paths;

        private Builder() {
        }

        /**
         * Set the fully-qualified domain name of the certificate that should be generated.
         *
         * @param fqdn The FQDN
         * @return This builder
         */
        public Builder fqdn(String fqdn) {
            this.fqdn = ObjectUtil.checkNotNullWithIAE(fqdn, "fqdn");
            return this;
        }

        /**
         * Set the RNG to use for key generation. This setting is not supported by the keytool-based generator.
         *
         * @param random The CSPRNG
         * @return This builder
         */
        public Builder random(SecureRandom random) {
            this.random = random;
            return this;
        }

        /**
         * Set the key size.
         *
         * @param bits The key size
         * @return This builder
         */
        public Builder bits(int bits) {
            this.bits = bits;
            return this;
        }

        /**
         * Set the start of the certificate validity period.
         *
Repository: netty/netty
File: buffer/src/main/java/io/netty/buffer/PoolArenaMetric.java (Java)

Code before:

    /**
     * Return the number of tiny allocations done via the arena.
     *
     * @deprecated Tiny allocations have been merged into small allocations.
     */
    @Deprecated
    long numTinyAllocations();

    /**
     * Return the number of small allocations done via the arena.
     */
    long numSmallAllocations();

    /**
     * Return the number of normal allocations done via the arena.
     */
    long numNormalAllocations();

    /**
     * Return the number of huge allocations done via the arena.
     */
    long numHugeAllocations();

    /**
     * Return the number of chunks allocations done via the arena.
     */
    default long numChunkAllocations() {
        return 0;
    }

    /**
     * Return the number of deallocations done via the arena. This includes all sizes.
     */
    long numDeallocations();

    /**
     * Return the number of tiny deallocations done via the arena.
     *
     * @deprecated Tiny deallocations have been merged into small deallocations.
     */
    @Deprecated
    long numTinyDeallocations();

    /**
     * Return the number of small deallocations done via the arena.
     */
    long numSmallDeallocations();

    /**
     * Return the number of normal deallocations done via the arena.

Review comment (suggestion):
Maybe we should return -1 by default and add to the javadocs that -1 is returned when "unknown" ? Same for all the other methods. 

Code after:

    /**
     * Return the number of tiny allocations done via the arena.
     *
     * @deprecated Tiny allocations have been merged into small allocations.
     */
    @Deprecated
    long numTinyAllocations();

    /**
     * Return the number of small allocations done via the arena.
     */
    long numSmallAllocations();

    /**
     * Return the number of normal allocations done via the arena.
     */
    long numNormalAllocations();

    /**
     * Return the number of huge allocations done via the arena.
     */
    long numHugeAllocations();

    /**
     * Return the number of chunks allocations done via the arena, or -1 if not defined.
     */
    default long numChunkAllocations() {
        return -1;
    }

    /**
     * Return the number of deallocations done via the arena. This includes all sizes.
     */
    long numDeallocations();

    /**
     * Return the number of tiny deallocations done via the arena.
     *
     * @deprecated Tiny deallocations have been merged into small deallocations.
     */
    @Deprecated
    long numTinyDeallocations();

    /**
     * Return the number of small deallocations done via the arena.
     */
    long numSmallDeallocations();

    /**
     * Return the number of normal deallocations done via the arena.
Repository: netty/netty
File: common/src/main/java/io/netty/util/internal/BoundedInputStream.java (Java)

Code before:

    public BoundedInputStream(@NotNull InputStream in) {
        this(in, 8 * 1024);
    }

    @Override
    public int read() throws IOException {
        checkMaxBytesRead(1);
        try {
            int b = super.read();
            if (b <= 0) {
                // We couldn't read anything.
                numRead--;
            }
            return b;
        } catch (IOException e) {
            numRead--;
            throw e;
        }
    }

    @Override
    public int read(byte[] buf, int off, int len) throws IOException {
        // Calculate the maximum number of bytes that we should try to read.
        int num = Math.min(len, maxBytesRead - numRead + 1);
        checkMaxBytesRead(num);
        try {
            int b = super.read(buf, off, num);
            if (b == -1) {
                // We couldn't read anything.
                numRead -= num;
            } else if (b != num) {
                // Correct numRead based on the actual amount we were able to read.
                numRead -= num - b;
            }
            return b;
        } catch (IOException e) {
            numRead -= num;
            throw e;
        }
    }

    private void checkMaxBytesRead(int n) throws IOException {
        int sum = numRead + n;
        if (sum < 0 || sum > maxBytesRead) {
            numRead = maxBytesRead + 1;
            throw new IOException("Maximum number of bytes read: " + maxBytesRead);
        }
        numRead = sum;
    }
}

Review comment (bug):
The original motivation for checking for `num` was to fail before starting the actual read (no need to try reading if we know we are going out of bound). Is it possible to preserve that behavior?

Code after:

    public BoundedInputStream(@NotNull InputStream in) {
        this(in, 8 * 1024);
    }

    @Override
    public int read() throws IOException {
        checkMaxBytesRead();

        int b = super.read();
        if (b > 0) {
            numRead++;
        }

        checkMaxBytesRead();
        return b;
    }

    @Override
    public int read(byte[] buf, int off, int len) throws IOException {
        checkMaxBytesRead();

        // Calculate the maximum number of bytes that we should try to read.
        int num = Math.min(len, maxBytesRead - numRead + 1);

        int b = super.read(buf, off, num);

        if (b > 0) {
            numRead += b;
        }

        checkMaxBytesRead();
        return b;
    }

    private void checkMaxBytesRead() throws IOException {
        if (numRead > maxBytesRead) {
            throw new IOException("Maximum number of bytes read: " + numRead);
        }
    }
}
Repository: netty/netty
File: buffer/src/main/java/io/netty/buffer/AdaptivePoolingAllocator.java (Java)

Code before:
        private volatile int refCnt;

        Chunk() {
            // Constructor only used by the MAGAZINE_FREED sentinel.
            delegate = null;
            magazine = null;
            allocator = null;
            capacity = 0;
            pooled = false;
        }

        Chunk(AbstractByteBuf delegate, Magazine magazine, boolean pooled) {
            this.delegate = delegate;
            this.pooled = pooled;
            capacity = delegate.capacity();
            updater.setInitialValue(this);
            allocator = magazine.parent;
            attachToMagazine(magazine);
        }

        Magazine currentMagazine()  {
            return magazine;
        }

        void detachFromMagazine() {
            magazine.usedMemory.getAndAdd(-capacity);
            magazine = null;
        }

        void attachToMagazine(Magazine magazine) {
            assert this.magazine == null;
            this.magazine = magazine;
            magazine.usedMemory.getAndAdd(capacity);
        }

        @Override
        public Chunk touch(Object hint) {
            return this;
        }

        @Override
        public int refCnt() {
            return updater.refCnt(this);
        }

        @Override
        public Chunk retain() {
            return updater.retain(this);
        }

        @Override

Review comment (suggestion):
Only do this if `magazine` is not already `null`. This can happen through a line of calls that start at `freeCentralQueue()`.

Code after:
        private volatile int refCnt;

        Chunk() {
            // Constructor only used by the MAGAZINE_FREED sentinel.
            delegate = null;
            magazine = null;
            allocator = null;
            capacity = 0;
            pooled = false;
        }

        Chunk(AbstractByteBuf delegate, Magazine magazine, boolean pooled) {
            this.delegate = delegate;
            this.pooled = pooled;
            capacity = delegate.capacity();
            updater.setInitialValue(this);
            allocator = magazine.parent;
            attachToMagazine(magazine);
        }

        Magazine currentMagazine()  {
            return magazine;
        }

        void detachFromMagazine() {
            if (magazine != null) {
                magazine.usedMemory.getAndAdd(-capacity);
                magazine = null;
            }
        }

        void attachToMagazine(Magazine magazine) {
            assert this.magazine == null;
            this.magazine = magazine;
            magazine.usedMemory.getAndAdd(capacity);
        }

        @Override
        public Chunk touch(Object hint) {
            return this;
        }

        @Override
        public int refCnt() {
            return updater.refCnt(this);
        }

        @Override
        public Chunk retain() {
            return updater.retain(this);
        }
Repository: netty/netty
File: transport-classes-io_uring/src/main/java/io/netty/channel/uring/IoUringIoOps.java (Java)

Code before:
     * Returns the priority.
     *
     * @return ioPrio
     */
    public short ioPrio() {
        return ioPrio;
    }

    /**
     * Returns the rwFlags that will be applied. These are specific to the opcode.
     *
     * @return  rwFlags
     */
    public int rwFlags() {
        return rwFlags;
    }

    /**
     * Returns the bufferAddress that will be used. This is specific to the opcode.
     *
     * @return  bufferAddress
     */
    public long bufferAddress() {
        return bufferAddress;
    }

    /**
     * Returns the length that will be used. This is specific to the opcode.
     *
     * @return  length
     */
    public int length() {
        return length;
    }

    /**
     * Returns the offset that will be used. This is specific to the opcode.
     *
     * @return  offset
     */
    public long offset() {
        return offset;
    }

    /**
     * Returns the data that the user attached to the op. This data will be passed back on completion.
     *
     * @return  data
     */
    public short data() {
        return data;

Review comment (suggestion):
I used a long for this to keep it simple. If we need something more advanced later we can add another constructor that takes a byte[].

Code after:
     *      };
     *      __u64 user_data;    // data to be passed back at completion time
     *                          // pack this to avoid bogus arm OABI complaints
     *
     *      union {             // union4
     *
     *          // index into fixed buffers, if used
     *          __u16 buf_index;
     *          // for grouped buffer selection
     *          __u16 buf_group;
     *      }__attribute__((packed));
     *      // personality to use, if used
     *      __u16 personality;
     *
     *      union {            // union5
     *
     *          __s32 splice_fd_in;
     *          __u32 file_index;
     *          __u32 optlen;
     *          struct {
     *              __u16 addr_len;
     *              __u16 __pad3[ 1];
     *          };
     *      };
     *
     *      union {           // union6
     *
     *          struct {
     *              __u64 addr3;
     *              __u64 __pad2[ 1];
     *          };
     *          __u64 optval;
     *          //
     *          // If the ring is initialized with IORING_SETUP_SQE128, then
     *          // this field is used for 80 bytes of arbitrary command data
     *          __u8 cmd[ 0];
     *      };
     *  };
     * }
     * </pre>
     */
    public IoUringIoOps(byte opcode, byte flags, short ioPrio, int fd, long union1, long union2, int len, int union3,
                        short data, short union4, short personality, int union5, long union6) {
        this.opcode = opcode;
        this.flags = flags;
        this.ioPrio = ioPrio;
        this.fd = fd;
        this.union1 = union1;
        this.union2 = union2;
        this.len = len;
        this.union3 = union3;
Repository: netty/netty
File: transport-classes-io_uring/src/main/java/io/netty/channel/uring/AbstractIoUringChannel.java (Java)

Code before:
        if (requestedRemoteAddress instanceof InetSocketAddress) {
            remote = computeRemoteAddr((InetSocketAddress) requestedRemoteAddress, socket.remoteAddress());
        }
    }

    private boolean shouldBreakIoUringInReady(ChannelConfig config) {
        return socket.isInputShutdown() && (inputClosedSeenErrorOnRead || !isAllowHalfClosure(config));
    }

    private void clearPollFlag(int pollMask) {
        if ((pollMask & Native.POLLIN) != 0) {
            ioState &= ~POLL_IN_SCHEDULED;
            pollInId = 0;
        }
        if ((pollMask & Native.POLLOUT) != 0) {
            ioState &= ~POLL_OUT_SCHEDULED;
            pollOutId = 0;
        }
        if ((pollMask & Native.POLLRDHUP) != 0) {
            ioState &= ~POLL_RDHUP_SCHEDULED;
            pollRdhupId = 0;
        }
     }

    static boolean socketWasEmptyForSure(int flags) {
        return IoUring.isIOUringCqeFSockNonEmptySupported() &&  (flags & Native.IORING_CQE_F_SOCK_NONEMPTY) == 0;
    }
}


Review comment (suggestion):
```suggestion
        return IoUring.isIOUringCqeFSockNonEmptySupported() && (flags & Native.IORING_CQE_F_SOCK_NONEMPTY) == 0;
```

Code after:
            connectTimeoutFuture = null;
        }
    }

    private void computeRemote() {
        if (requestedRemoteAddress instanceof InetSocketAddress) {
            remote = computeRemoteAddr((InetSocketAddress) requestedRemoteAddress, socket.remoteAddress());
        }
    }

    private boolean shouldBreakIoUringInReady(ChannelConfig config) {
        return socket.isInputShutdown() && (inputClosedSeenErrorOnRead || !isAllowHalfClosure(config));
    }

    private void clearPollFlag(int pollMask) {
        if ((pollMask & Native.POLLIN) != 0) {
            ioState &= ~POLL_IN_SCHEDULED;
            pollInId = 0;
        }
        if ((pollMask & Native.POLLOUT) != 0) {
            ioState &= ~POLL_OUT_SCHEDULED;
            pollOutId = 0;
        }
        if ((pollMask & Native.POLLRDHUP) != 0) {
            ioState &= ~POLL_RDHUP_SCHEDULED;
            pollRdhupId = 0;
        }
     }

    protected static boolean socketIsEmpty(int flags) {
        return IoUring.isIOUringCqeFSockNonEmptySupported() && (flags & Native.IORING_CQE_F_SOCK_NONEMPTY) == 0;
    }
}
Repository: netty/netty
File: codec/src/main/java/io/netty/handler/codec/compression/BrotliMode.java (Java)

Code before:
/*
 * Copyright 2021 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.handler.codec.compression;

import com.aayushatharva.brotli4j.encoder.Encoder;

import static com.aayushatharva.brotli4j.encoder.Encoder.Mode;

/**
 *  Provides an interface to specify the Brotli compression mode.
 *
 *   @author raccoonback
 */
public enum BrotliMode {

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.handler.codec.compression;

import com.aayushatharva.brotli4j.encoder.Encoder;

import static com.aayushatharva.brotli4j.encoder.Encoder.Mode;

/**
 * Provides a way to specify the Brotli compression mode.
 */
public enum BrotliMode {

    /**
Repository: netty/netty
File: codec/src/main/java/io/netty/handler/codec/compression/BrotliMode.java (Java)

Code before:
/*
 * Copyright 2021 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.handler.codec.compression;

import com.aayushatharva.brotli4j.encoder.Encoder;

import static com.aayushatharva.brotli4j.encoder.Encoder.Mode;

/**
 *  Provides an interface to specify the Brotli compression mode.
 *
 *   @author raccoonback
 */
public enum BrotliMode {

	/**
	 *  The compressor does not make any assumptions about the input data's properties,
	 *  making it suitable for a wide range of data types.
	 *  default mode.
	 */
	GENERIC(Mode.GENERIC),

	/**
	 *  Optimized for UTF-8 formatted text input.
	 */
	TEXT(Mode.TEXT),

	/**
	 *  Designed specifically for font data compression, as used in WOFF 2.0.
	 */
	FONT(Mode.FONT);

	private final Mode encoderMode;

	BrotliMode(Mode encoderMode) {

Review comment (suggestion):
```suggestion
 *  Provides a way to specify the Brotli compression mode.
```

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.handler.codec.compression;

import com.aayushatharva.brotli4j.encoder.Encoder;

import static com.aayushatharva.brotli4j.encoder.Encoder.Mode;

/**
 * Provides a way to specify the Brotli compression mode.
 */
public enum BrotliMode {

    /**
     * The compressor does not make any assumptions about the input data's properties,
     * making it suitable for a wide range of data types.
     * default mode.
     */
    GENERIC,

    /**
     * Optimized for UTF-8 formatted text input.
     */
    TEXT,

    /**
     * Designed specifically for font data compression, as used in WOFF 2.0.
     */
    FONT;

    /**
     * Convert to Brotli {@link Encoder.Mode}.
     *
     * @return a new {@link Encoder.Mode}
     */
Repository: netty/netty
File: codec/src/main/java/io/netty/handler/codec/compression/BrotliMode.java (Java)

Code before:

	/**
	 *  Optimized for UTF-8 formatted text input.
	 */
	TEXT(Mode.TEXT),

	/**
	 *  Designed specifically for font data compression, as used in WOFF 2.0.
	 */
	FONT(Mode.FONT);

	private final Mode encoderMode;

	BrotliMode(Mode encoderMode) {
		this.encoderMode = encoderMode;
	}


	/**
	 *  Convert to Brotli {@link Encoder.Mode}.
	 *
	 * @return a new {@link Encoder.Mode}
	 */
	Mode adapt() {
		return encoderMode;
	}
}


Review comment (performance):
I think this would be better:

```suggestion
public enum BrotliMode {

	/**
	 *  The compressor does not make any assumptions about the input data's properties,
	 *  making it suitable for a wide range of data types.
	 *  default mode.
	 */
	GENERIC,

	/**
	 *  Optimized for UTF-8 formatted text input.
	 */
	TEXT,

	/**
	 *  Designed specifically for font data compression, as used in WOFF 2.0.
	 */
	FONT;

	/**
	 *  Convert to Brotli {@link Encoder.Mode}.
	 *
	 * @return a new {@link Encoder.Mode}
	 */
	Mode adapt() {
		switch (this) {
		    case GENERIC:
		        return Mode.GENERIC;
		    case TEXT:
		        return Mode.TEXT;
		    case FONT:
		        return Mode.FONT;
		    default:
		        throw new IllegalStateException();
		}
	}
```

Code after:
     * Optimized for UTF-8 formatted text input.
     */
    TEXT,

    /**
     * Designed specifically for font data compression, as used in WOFF 2.0.
     */
    FONT;

    /**
     * Convert to Brotli {@link Encoder.Mode}.
     *
     * @return a new {@link Encoder.Mode}
     */
    Mode adapt() {
        switch (this) {
            case GENERIC:
                return Mode.GENERIC;
            case TEXT:
                return Mode.TEXT;
            case FONT:
                return Mode.FONT;
            default:
                throw new IllegalStateException("Unsupported enum value: " + this);
        }
    }
}