You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/02/01 12:07:39 UTC
[GitHub] [pulsar] xxxxpenny opened a new pull request #9400: op.cmd may be null, cause NPE
xxxxpenny opened a new pull request #9400:
URL: https://github.com/apache/pulsar/pull/9400
check msg = op.cms == null, so use op.cmd. readableBytes() may throw NPE
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #9400: op.cmd may be null, cause NPE
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #9400:
URL: https://github.com/apache/pulsar/pull/9400#issuecomment-770856786
I left one suggestion.
Can you please change the title pointing out that we are in the Producer, in the Java Client ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] xxxxpenny edited a comment on pull request #9400: [java producer] op.cmd may be null, cause NPE
Posted by GitBox <gi...@apache.org>.
xxxxpenny edited a comment on pull request #9400:
URL: https://github.com/apache/pulsar/pull/9400#issuecomment-770878106
yeah,I change the title.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] xxxxpenny commented on pull request #9400: [java producer] op.cmd may be null, cause NPE
Posted by GitBox <gi...@apache.org>.
xxxxpenny commented on pull request #9400:
URL: https://github.com/apache/pulsar/pull/9400#issuecomment-770878106
yeah，I change the title.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] xxxxpenny edited a comment on pull request #9400: op.cmd may be null, cause NPE
Posted by GitBox <gi...@apache.org>.
xxxxpenny edited a comment on pull request #9400:
URL: https://github.com/apache/pulsar/pull/9400#issuecomment-770854851
No, just a static analysis of code that led me to this patch。
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat merged pull request #9400: [java producer] op.cmd may be null, cause NPE
Posted by GitBox <gi...@apache.org>.
merlimat merged pull request #9400:
URL: https://github.com/apache/pulsar/pull/9400
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat merged pull request #9400: [java producer] op.cmd may be null, cause NPE
Posted by GitBox <gi...@apache.org>.
merlimat merged pull request #9400:
URL: https://github.com/apache/pulsar/pull/9400
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a change in pull request #9400: op.cmd may be null, cause NPE
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #9400:
URL: https://github.com/apache/pulsar/pull/9400#discussion_r567823637
##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
##########
@@ -1518,8 +1518,7 @@ private void stripChecksum(OpSendMsg op) {
headerFrame.resetReaderIndex();
}
} else {
- log.warn("[{}] Failed while casting {} into ByteBufPair", producerName,
- (op.cmd == null ? null : op.cmd.getClass().getName()));
+ log.warn("[{}] Failed while casting {} into ByteBufPair", producerName, null);
Review comment:
here you do not need to pass `null`, you can simply write `null` in the message
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #9400: op.cmd may be null, cause NPE
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #9400:
URL: https://github.com/apache/pulsar/pull/9400#issuecomment-770835273
Do have seen this problem in production ? or is it just a static analysis of code that led you to this patch ?
thanks for sharing your fix
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] xxxxpenny commented on pull request #9400: op.cmd may be null, cause NPE
Posted by GitBox <gi...@apache.org>.
xxxxpenny commented on pull request #9400:
URL: https://github.com/apache/pulsar/pull/9400#issuecomment-770854851
No, just a static analysis of code that led you to this patch。
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org