feedback-to-code/mozzarella huggingface dataset:
Repository: netty/netty
Issue: Wrong the comment for the remove method on FastThreadLocal class.

https://github.com/netty/netty/blob/0dc246eb129796313b58c1dbdd674aa289f72cad/common/src/main/java/io/netty/util/concurrent/FastThreadLocal.java#L234

Issue comments:
What's wrong about it?
`    /**
     * Sets the value to uninitialized; a proceeding call to get() will trigger a call to initialValue().
     */
    public final void remove() {
        remove(InternalThreadLocalMap.getIfSet());
    }`

@chrisvest,
The remove() method call to getIfSet() not call to get().

What it's meant to say is that after you've called `remove()`, if you then call `get()`, you'll get a new `initialValue()` out.
@chrisvest,
I got it. I think to more meant should change the word `proceeding` by other words.


Pull request review comments:

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/concurrent/FastThreadLocal.java b/common/src/main/java/io/netty/util/concurrent/FastThreadLocal.java
index 9b31dd4504e..750b12d8b49 100644
--- a/common/src/main/java/io/netty/util/concurrent/FastThreadLocal.java
+++ b/common/src/main/java/io/netty/util/concurrent/FastThreadLocal.java
@@ -208,7 +208,7 @@ public final void set(InternalThreadLocalMap threadLocalMap, V value) {
     }
 
     /**
-     * @return see {@link InternalThreadLocalMap#setIndexedVariable(int, Object)}.
+     * @see InternalThreadLocalMap#setIndexedVariable(int, Object).
      */
     private void setKnownNotUnset(InternalThreadLocalMap threadLocalMap, V value) {
         if (threadLocalMap.setIndexedVariable(index, value)) {
@@ -231,15 +231,16 @@ public final boolean isSet(InternalThreadLocalMap threadLocalMap) {
         return threadLocalMap != null && threadLocalMap.isIndexedVariableSet(index);
     }
     /**
-     * Sets the value to uninitialized; a proceeding call to get() will trigger a call to initialValue().
+     * Sets the value to uninitialized for the specified thread local map.
+     * After this, any subsequent call to get() will trigger a new call to initialValue().
      */
     public final void remove() {
         remove(InternalThreadLocalMap.getIfSet());
     }
 
     /**
-     * Sets the value to uninitialized for the specified thread local map;
-     * a proceeding call to get() will trigger a call to initialValue().
+     * Sets the value to uninitialized for the specified thread local map.
+     * After this, any subsequent call to get() will trigger a new call to initialValue().
      * The specified thread local map must be for the current thread.
      */
     @SuppressWarnings("unchecked")


Test patch:
Repository: netty/netty
Issue: Doc incorrect at io.netty.util.concurrent.AbstractEventExecutor#lazyExecute

### Actual behavior
```
    /**
     * Like {@link #execute(Runnable)} but does not guarantee the task will be run until either
     * a non-lazy task is executed or the executor is shut down.
     *
     * This is equivalent to submitting a {@link EventExecutor.LazyRunnable} to
     * {@link #execute(Runnable)} but for an arbitrary {@link Runnable}.
     *
     * The default implementation just delegates to {@link #execute(Runnable)}.
     */
    @UnstableApi
    public void lazyExecute(Runnable task) {
        execute(task);
    }
```
The lazyRunnable already isn't belong to EventExecutor anymore.


### Netty version
branch 4.1


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/concurrent/AbstractEventExecutor.java b/common/src/main/java/io/netty/util/concurrent/AbstractEventExecutor.java
index b1df38a81aa..e4ca3440960 100644
--- a/common/src/main/java/io/netty/util/concurrent/AbstractEventExecutor.java
+++ b/common/src/main/java/io/netty/util/concurrent/AbstractEventExecutor.java
@@ -171,7 +171,7 @@ protected static void safeExecute(Runnable task) {
      * Like {@link #execute(Runnable)} but does not guarantee the task will be run until either
      * a non-lazy task is executed or the executor is shut down.
      *
-     * This is equivalent to submitting a {@link EventExecutor.LazyRunnable} to
+     * This is equivalent to submitting a {@link AbstractEventExecutor.LazyRunnable} to
      * {@link #execute(Runnable)} but for an arbitrary {@link Runnable}.
      *
      * The default implementation just delegates to {@link #execute(Runnable)}.


Test patch:
Repository: netty/netty
Issue: Code clean at io.netty.buffer.AbstractByteBuf#adjustMarkers

https://github.com/netty/netty/blob/28d4154fffc9c51f73b5969c64d74ca17ca4baba/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java#L257-L271

It's unnecessary to create stack variable. 



Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
index c13efdd0046..fb152502719 100644
--- a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
@@ -255,17 +255,15 @@ public ByteBuf discardSomeReadBytes() {
     }
 
     protected final void adjustMarkers(int decrement) {
-        int markedReaderIndex = this.markedReaderIndex;
         if (markedReaderIndex <= decrement) {
-            this.markedReaderIndex = 0;
-            int markedWriterIndex = this.markedWriterIndex;
+            markedReaderIndex = 0;
             if (markedWriterIndex <= decrement) {
-                this.markedWriterIndex = 0;
+                markedWriterIndex = 0;
             } else {
-                this.markedWriterIndex = markedWriterIndex - decrement;
+                markedWriterIndex -= decrement;
             }
         } else {
-            this.markedReaderIndex = markedReaderIndex - decrement;
+            markedReaderIndex -= decrement;
             markedWriterIndex -= decrement;
         }
     }


Test patch:
Repository: netty/netty
Issue: ChunkedInputStream isEndOfInput blocks event loop

In all versions of netty the ChunkedInputStream isEndOfInput uses b.read() which is blocking. This should instead use available()

Issue comments:
`available()` only tells you how many bytes you can read without blocking; it doesn't tell if you're at the end of the stream.
Actually it returns -1 or an error when at end of stream

Pull request review comments:

Patch that resolved the issue:
diff --git a/handler/src/main/java/io/netty/handler/stream/ChunkedStream.java b/handler/src/main/java/io/netty/handler/stream/ChunkedStream.java
index 8f67df20f8d..86e3f5d5c71 100644
--- a/handler/src/main/java/io/netty/handler/stream/ChunkedStream.java
+++ b/handler/src/main/java/io/netty/handler/stream/ChunkedStream.java
@@ -79,6 +79,9 @@ public boolean isEndOfInput() throws Exception {
         if (closed) {
             return true;
         }
+        if (in.available() > 0) {
+            return false;
+        }
 
         int b = in.read();
         if (b < 0) {


Test patch:
diff --git a/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketChannelConfigTest.java b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketChannelConfigTest.java
index a859085aeae..299183d89d6 100644
--- a/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketChannelConfigTest.java
+++ b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketChannelConfigTest.java
@@ -155,7 +155,7 @@ public void testGetOptionWhenClosed() {
             if (!(e.getCause() instanceof ClosedChannelException)) {
                 AssertionError error = new AssertionError(
                         "Expected the suppressed exception to be an instance of ClosedChannelException.");
-                error.addSuppressed(e.getCause());
+                error.addSuppressed(e);
                 throw error;
             }
         }
Repository: netty/netty
Issue: Remove dependency on log4J 1.x branch

Netty, and thus all downstream projects (e.g. vert.x, Quarkus etc) show a dependency on log4j-1.2.17 which as been flagged with CVE: 20-12-2019. From a vulnerability scan:

> Severity: High
>
> CVE: 20-12-2019
>
> Description: Included in Log4j 1.2 is a SocketServer class that is vulnerable to deserialization of untrusted data which can be exploited to remotely execute arbitrary code when combined with a deserialization gadget when listening to untrusted network traffic for log data. This affects Log4j versions up to 1.2 up to 1.2.17.
>
> Published: 20-12-2019
>
> Top Fix: 
> Upgrade to version org.apache.logging.log4j:log4j-core:2.0.  The reported vulnerability was not checked for vulnerability effectiveness and is suggested to be examined using Effective Usage Analysis.

There are 2 occurrences that need upgrade:

https://github.com/netty/netty/blob/ccca3959fd64b133edca855925fb68d077c3a8a2/common/src/main/java/io/netty/util/internal/logging/Log4JLoggerFactory.java#L18

Replace with:

```java
import org.apache.logging.log4j.Logger;
```

https://github.com/netty/netty/blob/d34212439068091bcec29a8fad4df82f0a82c638/common/src/main/java/io/netty/util/internal/logging/Log4JLogger.java#L42-L43

Replace with:

```java
import org.apache.logger.log4j.Level;
import org.apache.logger.log4j.Logger;
```


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/common/pom.xml b/common/pom.xml
index 15a5dc12679..a930b94f1bd 100644
--- a/common/pom.xml
+++ b/common/pom.xml
@@ -64,8 +64,8 @@
       <optional>true</optional>
     </dependency>
     <dependency>
-      <groupId>log4j</groupId>
-      <artifactId>log4j</artifactId>
+      <groupId>org.apache.logging.log4j</groupId>
+      <artifactId>log4j-1.2-api</artifactId>
       <optional>true</optional>
     </dependency>
     <dependency>
diff --git a/pom.xml b/pom.xml
index e677a89ff05..1478851ace8 100644
--- a/pom.xml
+++ b/pom.xml
@@ -728,9 +728,9 @@
         <version>${log4j2.version}</version>
       </dependency>
       <dependency>
-        <groupId>log4j</groupId>
-        <artifactId>log4j</artifactId>
-        <version>1.2.17</version>
+        <groupId>org.apache.logging.log4j</groupId>
+        <artifactId>log4j-1.2-api</artifactId>
+        <version>2.14.1</version>
         <exclusions>
           <exclusion>
             <artifactId>mail</artifactId>


Test patch:
Repository: netty/netty
Issue: Method 'io.netty.util.internal.MathUtil#isOutOfBounds' needs to check the 'capacity' parameter

### Expected behavior
As `io.netty.util.internal.MathUtil#isOutOfBounds` is a public utility method, it should pass all kinds of test cases. Like:
```
import io.netty.util.internal.MathUtil;

public static void main(String[] args) {
        boolean isOutOfBounds = MathUtil.isOutOfBounds(1, 2, Integer.MIN_VALUE);
        System.out.println(isOutOfBounds);
}
```
It should print `true`, which means it is out of bound.

### Actual behavior
It printed `false`, which means it is NOT out of bound.
### Steps to reproduce
see code above.
### Minimal yet complete reproducer code (or URL to code)
see code above.
### Netty version
4.1.
### JVM version (e.g. `java -version`)
1.8.
### OS version (e.g. `uname -a`)
Darwin.

Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/internal/MathUtil.java b/common/src/main/java/io/netty/util/internal/MathUtil.java
index 68777e6d8e2..fd8e165eccf 100644
--- a/common/src/main/java/io/netty/util/internal/MathUtil.java
+++ b/common/src/main/java/io/netty/util/internal/MathUtil.java
@@ -57,11 +57,11 @@ public static int safeFindNextPositivePowerOfTwo(final int value) {
      * @param index The starting index.
      * @param length The length which will be utilized (starting from {@code index}).
      * @param capacity The capacity that {@code index + length} is allowed to be within.
-     * @return {@code true} if the requested {@code index} and {@code length} will fit within {@code capacity}.
-     * {@code false} if this would result in an index out of bounds exception.
+     * @return {@code false} if the requested {@code index} and {@code length} will fit within {@code capacity}.
+     * {@code true} if this would result in an index out of bounds exception.
      */
     public static boolean isOutOfBounds(int index, int length, int capacity) {
-        return (index | length | (index + length) | (capacity - (index + length))) < 0;
+        return (index | length | capacity | (index + length) | (capacity - (index + length))) < 0;
     }
 
     /**


Test patch:
diff --git a/common/src/test/java/io/netty/util/internal/MathUtilTest.java b/common/src/test/java/io/netty/util/internal/MathUtilTest.java
index 32628114dd9..2fd44ced4bc 100644
--- a/common/src/test/java/io/netty/util/internal/MathUtilTest.java
+++ b/common/src/test/java/io/netty/util/internal/MathUtilTest.java
@@ -68,6 +68,9 @@ public void testIsOutOfBounds() {
         assertTrue(isOutOfBounds(Integer.MAX_VALUE - 1, 1, Integer.MAX_VALUE - 1));
         assertTrue(isOutOfBounds(Integer.MAX_VALUE - 1, 2, Integer.MAX_VALUE));
         assertTrue(isOutOfBounds(1, Integer.MAX_VALUE, Integer.MAX_VALUE));
+        assertTrue(isOutOfBounds(0, 1, Integer.MIN_VALUE));
+        assertTrue(isOutOfBounds(0, 1, -1));
+        assertTrue(isOutOfBounds(0, Integer.MAX_VALUE, 0));
     }
 
     @Test
Repository: netty/netty
Issue: ability to CipherSuitesConverter outside of the io.netty.handler.ssl

In our use case, we are required to convert the java cipher suites are required to be converted to Openssl. I found that netty already has a `CipherSuitesConverter`. As we are already using netty, we could have reused the class if it was not made the package private. IMO it would be no harm if we make it public to be used for the project using netty already.

I will raise a pull request for the same.

Issue comments:

Pull request review comments:
revert ?
Not here... add it to `CipherSuiteConverter `
sorry, didn't understand this point. Failure which I see in the last build is : 
```
Failures: 
2021-09-04T19:13:09.0782449Z [ERROR]   JdkOpenSslEngineInteroptTest.testMutualAuthDiffCerts:78->SSLEngineTest.testMutualAuthDiffCerts:633->SSLEngineTest.runTest:1267->SSLEngineTest.writeAndVerifyReceived:1296 expected: <true> but was: <false>
```

But why we are adding `@UnstableApi` to `CipherSuitesConverter` class?

I see the point after reading the documentation about it.
updated.

Patch that resolved the issue:
diff --git a/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java b/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java
index 2dd81371adb..a9ac8307c1d 100644
--- a/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java
+++ b/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java
@@ -17,6 +17,7 @@
 package io.netty.handler.ssl;
 
 import io.netty.util.internal.PlatformDependent;
+import io.netty.util.internal.UnstableApi;
 import io.netty.util.internal.logging.InternalLogger;
 import io.netty.util.internal.logging.InternalLoggerFactory;
 
@@ -34,7 +35,8 @@
  *
  * @see <a href="https://en.wikipedia.org/wiki/Cipher_suite">Wikipedia page about cipher suite</a>
  */
-final class CipherSuiteConverter {
+@UnstableApi
+public final class CipherSuiteConverter {
 
     private static final InternalLogger logger = InternalLoggerFactory.getInstance(CipherSuiteConverter.class);
 
@@ -150,7 +152,7 @@ static boolean isO2JCached(String key, String protocol, String value) {
      *
      * @return {@code null} if the conversion has failed
      */
-    static String toOpenSsl(String javaCipherSuite, boolean boringSSL) {
+    public static String toOpenSsl(String javaCipherSuite, boolean boringSSL) {
         String converted = j2o.get(javaCipherSuite);
         if (converted != null) {
             return converted;
@@ -279,7 +281,7 @@ private static String toOpenSslHmacAlgo(String hmacAlgo) {
      * @param protocol The cryptographic protocol (i.e. SSL, TLS, ...).
      * @return The translated cipher suite name according to java conventions. This will not be {@code null}.
      */
-    static String toJava(String openSslCipherSuite, String protocol) {
+    public static String toJava(String openSslCipherSuite, String protocol) {
         Map<String, String> p2j = o2j.get(openSslCipherSuite);
         if (p2j == null) {
             p2j = cacheFromOpenSsl(openSslCipherSuite);


Test patch:
Repository: netty/netty
Issue: It has a problem that setCharSequence method description at ByteBuf.class

This is just a minor problem，I found that setCharSequence method description at ByteBuf.class。
```
    /**
     * Writes the specified {@link CharSequence} at the current {@code writerIndex} and increases
     * the {@code writerIndex} by the written bytes.
     *
     * @param index on which the sequence should be written
     * @param sequence to write
     * @param charset that should be used.
     * @return the written number of bytes.
     * @throws IndexOutOfBoundsException
     *         if {@code this.writableBytes} is not large enough to write the whole sequence
     */
    public abstract int setCharSequence(int index, CharSequence sequence, Charset charset);
```



Issue comments:
Hi @wo883721 

I read through the snippet you posted, but I don't see the issue. Can you explain a bit more?
@chrisvest  
I don't understand，Why can this method change the value of `writerIndex`?

`setCharSequence`  method implementation
```
    public int setCharSequence(int index, CharSequence sequence, Charset charset) {
        return setCharSequence0(index, sequence, charset, false);
    }
```

`writeCharSequence`  method implementation
```
    @Override
    public int writeCharSequence(CharSequence sequence, Charset charset) {
        int written = setCharSequence0(writerIndex, sequence, charset, true);
        writerIndex += written;
        return written;
    }
```
@wo883721 It's a "set" method. They take a target index as an argument, and leaves the bytebuf writer index unchanged.

If it changed the writer index, it would have to be a "write" method.
Oh I see. The javadocs have been copy-pasted from the write method. 

Pull request review comments:

Patch that resolved the issue:
diff --git a/buffer/src/main/java/io/netty/buffer/ByteBuf.java b/buffer/src/main/java/io/netty/buffer/ByteBuf.java
index fef30f125ae..908e110a57f 100644
--- a/buffer/src/main/java/io/netty/buffer/ByteBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/ByteBuf.java
@@ -1338,15 +1338,15 @@ public ByteBuf setDoubleLE(int index, double value) {
     public abstract ByteBuf setZero(int index, int length);
 
     /**
-     * Writes the specified {@link CharSequence} at the current {@code writerIndex} and increases
-     * the {@code writerIndex} by the written bytes.
+     * Writes the specified {@link CharSequence} at the given {@code index}.
+     * The {@code writerIndex} is not modified by this method.
      *
      * @param index on which the sequence should be written
      * @param sequence to write
      * @param charset that should be used.
      * @return the written number of bytes.
      * @throws IndexOutOfBoundsException
-     *         if {@code this.writableBytes} is not large enough to write the whole sequence
+     *         if the sequence at the given index would be out of bounds of the buffer capacity
      */
     public abstract int setCharSequence(int index, CharSequence sequence, Charset charset);
 


Test patch:
Repository: netty/netty
Issue: Clarify how to engage with the community

_Note: this is kind of a meta-issue, as it is not related to the code_

I have been working with Netty 4 for the past two months and it has been a great experience. At times I would have liked to talk to someone about design choices and general questions about the library, but I wasn't able to find a place where Netty users come together (e.g. an IRC channel, a Discord server, etc). Netty's website brought me to StackOverflow, but my impression was that most questions go unanswered. The website's link to IRC is broken, and when I checked out #netty on liberachat I only saw 3 users in the channel. This leaves me wondering: where does the Netty community hang around? It would be helpful to clarify this somewhere visible, because right now the first impression is that the project is not as active as it used to be.

By the way, in case you are curious, I implemented a MySQL proxy from scratch (i.e. it receives MySQL connections and forwards them to arbitrary backends). I have the feeling that the library has helped me follow good design practices and that it has provided me a solid foundation to build upon, so thanks for that 😉 !

Issue comments:
@normanmaurer I created the Netty Discord server a few days back. Should we make it public so Netty people can engage with each other?
I am just a (proud) Netty committer but I love the @hyperxpro idea 👍  
I saw many people struggling to find the right place to share their experience/hints...and with Netty 5 I believe this is going to happen more frequently 
@hyperxpro sure lets do it. 
Here we go: https://discord.gg/q4aQ2XjaCa

Let's spread the word and see how things go.

Pull request review comments:

Patch that resolved the issue:
diff --git a/README.md b/README.md
index 887e3c3ce13..0bf67c3c409 100644
--- a/README.md
+++ b/README.md
@@ -10,6 +10,7 @@ Netty is an asynchronous event-driven network application framework for rapid de
 * [Downloads](https://netty.io/downloads.html)
 * [Documentation](https://netty.io/wiki/)
 * [@netty_project](https://twitter.com/netty_project)
+* [Official Discord server](https://discord.gg/q4aQ2XjaCa)
 
 ## How to build
 


Test patch:
Repository: netty/netty
Issue: Internal Logging API - make MessageFormatter and FormattingTuple public

### Expected behavior
While trying to use netty with ZIO 2.x Logging support I found out that I can't implement properly AbstractInternalLogger  interface for integration with ZIO 2.x, relying on the optimised implementation of MessageFormatter.

### Actual behavior
Should be able to add new Logging integrations.

### Steps to reproduce

### Minimal yet complete reproducer code (or URL to code)

### Netty version
4.1.74
### JVM version (e.g. `java -version`)
Not dependent on java version
### OS version (e.g. `uname -a`)
Not dependent on OS version.

Issue comments:
@devsprint I wonder if it would be better to write a `SLF4J ` implement for it and then just use the provided SLF4J implementation for netty ?
To implement my current task, it is exactly what I did, but I was thinking of removing layers of indirections...

Pull request review comments:

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/internal/logging/FormattingTuple.java b/common/src/main/java/io/netty/util/internal/logging/FormattingTuple.java
index 59862691c82..c18a3a3b2ca 100644
--- a/common/src/main/java/io/netty/util/internal/logging/FormattingTuple.java
+++ b/common/src/main/java/io/netty/util/internal/logging/FormattingTuple.java
@@ -42,7 +42,7 @@
 /**
  * Holds the results of formatting done by {@link MessageFormatter}.
  */
-final class FormattingTuple {
+public final class FormattingTuple {
 
     private final String message;
     private final Throwable throwable;
diff --git a/common/src/main/java/io/netty/util/internal/logging/MessageFormatter.java b/common/src/main/java/io/netty/util/internal/logging/MessageFormatter.java
index 82f56dce069..4301c3adea6 100644
--- a/common/src/main/java/io/netty/util/internal/logging/MessageFormatter.java
+++ b/common/src/main/java/io/netty/util/internal/logging/MessageFormatter.java
@@ -108,7 +108,7 @@
  * {@link #format(String, Object, Object)} and
  * {@link #arrayFormat(String, Object[])} methods for more details.
  */
-final class MessageFormatter {
+public final class MessageFormatter {
     private static final String DELIM_STR = "{}";
     private static final char ESCAPE_CHAR = '\\';
 


Test patch: