You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by franz1981 <gi...@git.apache.org> on 2017/10/19 18:31:55 UTC

[GitHub] activemq-artemis pull request #1600: ARTEMIS-1471 Needs Bounds Checking on w...

GitHub user franz1981 opened a pull request:

    https://github.com/apache/activemq-artemis/pull/1600

    ARTEMIS-1471 Needs Bounds Checking on writes for MappedSequentialFile

    It contains several improvements:
     - bounds checking logic on writes and position change too (to avoid corrupted position/allow fail fast)
     - removal of unused package private methods
     - refactored private duplicated code in procedures

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/franz1981/activemq-artemis mmap_bounds_chk

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/1600.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1600
    
----
commit 7f4ebe61c60a3935a81101872e253340996f5253
Author: Francesco Nigro <ni...@gmail.com>
Date:   2017-10-19T18:17:34Z

    ARTEMIS-1471 Needs Bounds Checking on writes for MappedSequentialFile
    
    It contains several improvements:
     - bounds checking logic on writes and position change too (to avoid corrupted position/allow fail fast)
     - removal of unused package private methods
     - refactored private duplicated code in procedures

----


---

[GitHub] activemq-artemis pull request #1600: ARTEMIS-1471 Needs Bounds Checking on w...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/1600


---

[GitHub] activemq-artemis pull request #1600: ARTEMIS-1471 Needs Bounds Checking on w...

Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1600#discussion_r145978175
  
    --- Diff: artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/mapped/MappedFile.java ---
    @@ -301,23 +206,24 @@ public void zeros(int position, final int count) throws IOException {
           if (toZeros > 0) {
              PlatformDependent.setMemory(start, toZeros, (byte) 0);
           }
    -
    +      //do not move position!
    --- End diff --
    
    Nope, `this.position` isn't moved, only the local variable named `position` is: the correct comment could be `//do not move this.position`


---

[GitHub] activemq-artemis pull request #1600: ARTEMIS-1471 Needs Bounds Checking on w...

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1600#discussion_r145976943
  
    --- Diff: artemis-journal/src/main/java/org/apache/activemq/artemis/core/io/mapped/MappedFile.java ---
    @@ -301,23 +206,24 @@ public void zeros(int position, final int count) throws IOException {
           if (toZeros > 0) {
              PlatformDependent.setMemory(start, toZeros, (byte) 0);
           }
    -
    +      //do not move position!
    --- End diff --
    
    I don't understand this comment... you are moving position and writing do not move?


---