[ 
https://issues.apache.org/jira/browse/ARTEMIS-2931?focusedWorklogId=514715&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-514715
 ]

ASF GitHub Bot logged work on ARTEMIS-2931:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 20/Nov/20 17:02
            Start Date: 20/Nov/20 17:02
    Worklog Time Spent: 10m 
      Work Description: pekuz commented on a change in pull request #3295:
URL: https://github.com/apache/activemq-artemis/pull/3295#discussion_r527831108



##########
File path: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingStoreImpl.java
##########
@@ -565,11 +565,12 @@ public boolean checkPageFileExists(final int pageNumber) {
       SequentialFileFactory factory = null;
       try {
          factory = checkFileFactory();
+         SequentialFile file = factory.createSequentialFile(fileName);
+         return file.exists();
       } catch (Exception ignored) {
+         assert false : "PagingStoreFactory::newFileFactory never-throws 
assumption failed.";

Review comment:
       the assert gives flexibility:
      in prod environments it can erased (like it would not exist in the 
sources)
      in dev environments it stops so one have proof out of contact situation 
actually happened
   
   I went for assert as the contract is not evident. Returning mere false means 
that contract it clear and understood.  
   
   If anybody helps to clarify contract then removing assert is super simple 
change to accept. 
   
     Hope it helps




----------------------------------------------------------------
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:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 514715)
    Time Spent: 1h 10m  (was: 1h)

>  Potential null pointer access at several places
> ------------------------------------------------
>
>                 Key: ARTEMIS-2931
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-2931
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>          Components: Broker
>            Reporter: Petr Kuzel
>            Priority: Major
>          Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> Open the artemis-server project in Eclipse. Static analysis reports numerous 
> Potential null pointer access. Looking at an example, the static analyzer is 
> correct.
> ReplicationEndpoint:
> {code:java}
>    public void setChannel(final Channel channel) {
>       this.channel = channel;
>       if (channel == null) {
>          supportResponseBatching = false;
>       } else {
>          try {
>             final CoreRemotingConnection connection = channel.getConnection();
>             if (connection != null) {
>                this.supportResponseBatching = 
> connection.getTransportConnection() instanceof NettyConnection;
>             } else {
>                this.supportResponseBatching = false;
>             }
>          } catch (Throwable t) {
>             logger.warn("Error while checking the channel connection", t);
>             this.supportResponseBatching = false;
>          }
>       }      if (this.channel != null && outgoingInterceptors != null) {
>          if (channel.getConnection() instanceof RemotingConnectionImpl)  { // 
> NPE risk
> {code}
> line 392 the channel can be null.
> line 409 for unknown reason assumes the channel is never null.
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to