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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

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

asfgit
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 <[hidden email]>
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

----


---
Reply | Threaded
Open this post in threaded view
|

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

asfgit
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?


---
Reply | Threaded
Open this post in threaded view
|

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

asfgit
In reply to this post by asfgit
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`


---
Reply | Threaded
Open this post in threaded view
|

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

asfgit
In reply to this post by asfgit
Github user asfgit closed the pull request at:

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


---