You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Miguel Ferreira <mi...@me.com> on 2014/02/14 12:28:18 UTC

Review Request 18114: Fix findbugs findings in cloudstack-service-console-proxy-rdpclient

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18114/
-----------------------------------------------------------

Review request for cloudstack, daan Hoogland and Hugo Trippaers.


Repository: cloudstack-git


Description
-------

    Fix findbugs findings in cloudstack-service-console-proxy-rdpclient
    
    Findings:
    - 32 int shifted by an amount not in range -31..31:
        The shifts by 32 bits don't actually have any effect on the value
        (as shown by the tests)
    - possible null pointer dereference
    - repeated conditional test
    - field only ever set to null
        All other uses of the field were to check if it was null,
        which it was, so it was removed
    
    Other actions:
    - Upgrade jUnit to version 4
    - Add PowerMock dependency


Diffs
-----

  conditional PRE-CREATION 
  pom.xml 9dbd72b 
  services/console-proxy-rdp/rdpconsole/pom.xml 05585a1 
  services/console-proxy-rdp/rdpconsole/src/main/java/common/Client.java 688693b 
  services/console-proxy-rdp/rdpconsole/src/main/java/streamer/ByteBuffer.java 3a718ba 
  services/console-proxy-rdp/rdpconsole/src/test/java/common/ClientTest.java PRE-CREATION 
  services/console-proxy-rdp/rdpconsole/src/test/java/rdpclient/MockServerTest.java 17bc4bd 
  services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/18114/diff/


Testing
-------

Added tests for code that changed and tested in Devcloud


Thanks,

Miguel Ferreira


Re: Review Request 18114: Fix findbugs findings in cloudstack-service-console-proxy-rdpclient

Posted by Hugo Trippaers <ht...@schubergphilis.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18114/#review34482
-----------------------------------------------------------

Ship it!


commit 14a5751d0f4247efc6a982bf1c7e51408fff3894
Author: miguelaferreira <mf...@shubergphilis.com>
Date:   Tue Feb 11 18:56:05 2014 +0100

    Fix findbugs findings in cloudstack-service-console-proxy-rdpclient
    
    Findings:
    - 32 int shifted by an amount not in range -31..31:
        The shifts by 32 bits don't actually have any effect on the value
        (as shown by the tests)
    - possible null pointer dereference
    - repeated conditional test
    - field only ever set to null
        All other uses of the field were to check if it was null,
        which it was, so it was removed
    
    Other actions:
    - Upgrade jUnit to version 4
    - Add PowerMock dependency
    
    Signed-off-by: Hugo Trippaers <ht...@schubergphilis.com>


- Hugo Trippaers


On Feb. 14, 2014, 11:46 a.m., Miguel Ferreira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18114/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2014, 11:46 a.m.)
> 
> 
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
>     Fix findbugs findings in cloudstack-service-console-proxy-rdpclient
>     
>     Findings:
>     - 32 int shifted by an amount not in range -31..31:
>         The shifts by 32 bits don't actually have any effect on the value
>         (as shown by the tests)
>     - possible null pointer dereference
>     - repeated conditional test
>     - field only ever set to null
>         All other uses of the field were to check if it was null,
>         which it was, so it was removed
>     
>     Other actions:
>     - Upgrade jUnit to version 4
>     - Add PowerMock dependency
> 
> 
> Diffs
> -----
> 
>   pom.xml 9dbd72b 
>   services/console-proxy-rdp/rdpconsole/pom.xml 05585a1 
>   services/console-proxy-rdp/rdpconsole/src/main/java/common/Client.java 688693b 
>   services/console-proxy-rdp/rdpconsole/src/main/java/streamer/ByteBuffer.java 3a718ba 
>   services/console-proxy-rdp/rdpconsole/src/test/java/common/ClientTest.java PRE-CREATION 
>   services/console-proxy-rdp/rdpconsole/src/test/java/rdpclient/MockServerTest.java 17bc4bd 
>   services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18114/diff/
> 
> 
> Testing
> -------
> 
> Added tests for code that changed and tested in Devcloud
> 
> 
> Thanks,
> 
> Miguel Ferreira
> 
>


Re: Review Request 18114: Fix findbugs findings in cloudstack-service-console-proxy-rdpclient

Posted by Hugo Trippaers <ht...@schubergphilis.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18114/#review34483
-----------------------------------------------------------

Ship it!


commit 14a5751d0f4247efc6a982bf1c7e51408fff3894
Author: miguelaferreira <mf...@shubergphilis.com>
Date:   Tue Feb 11 18:56:05 2014 +0100

    Fix findbugs findings in cloudstack-service-console-proxy-rdpclient
    
    Findings:
    - 32 int shifted by an amount not in range -31..31:
        The shifts by 32 bits don't actually have any effect on the value
        (as shown by the tests)
    - possible null pointer dereference
    - repeated conditional test
    - field only ever set to null
        All other uses of the field were to check if it was null,
        which it was, so it was removed
    
    Other actions:
    - Upgrade jUnit to version 4
    - Add PowerMock dependency
    
    Signed-off-by: Hugo Trippaers <ht...@schubergphilis.com>


- Hugo Trippaers


On Feb. 14, 2014, 11:46 a.m., Miguel Ferreira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18114/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2014, 11:46 a.m.)
> 
> 
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
>     Fix findbugs findings in cloudstack-service-console-proxy-rdpclient
>     
>     Findings:
>     - 32 int shifted by an amount not in range -31..31:
>         The shifts by 32 bits don't actually have any effect on the value
>         (as shown by the tests)
>     - possible null pointer dereference
>     - repeated conditional test
>     - field only ever set to null
>         All other uses of the field were to check if it was null,
>         which it was, so it was removed
>     
>     Other actions:
>     - Upgrade jUnit to version 4
>     - Add PowerMock dependency
> 
> 
> Diffs
> -----
> 
>   pom.xml 9dbd72b 
>   services/console-proxy-rdp/rdpconsole/pom.xml 05585a1 
>   services/console-proxy-rdp/rdpconsole/src/main/java/common/Client.java 688693b 
>   services/console-proxy-rdp/rdpconsole/src/main/java/streamer/ByteBuffer.java 3a718ba 
>   services/console-proxy-rdp/rdpconsole/src/test/java/common/ClientTest.java PRE-CREATION 
>   services/console-proxy-rdp/rdpconsole/src/test/java/rdpclient/MockServerTest.java 17bc4bd 
>   services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18114/diff/
> 
> 
> Testing
> -------
> 
> Added tests for code that changed and tested in Devcloud
> 
> 
> Thanks,
> 
> Miguel Ferreira
> 
>


Re: Review Request 18114: Fix findbugs findings in cloudstack-service-console-proxy-rdpclient

Posted by Miguel Ferreira <mi...@me.com>.

> On Feb. 19, 2014, 7:34 a.m., Hugo Trippaers wrote:
> > Miguel, can you check the last version of your patch versus current master? Anything we missed?

I did review the patch. I don't see anything missing. Why do you ask?


- Miguel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18114/#review34851
-----------------------------------------------------------


On Feb. 14, 2014, 2:48 p.m., Miguel Ferreira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18114/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2014, 2:48 p.m.)
> 
> 
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
>     Fix findbugs findings in cloudstack-service-console-proxy-rdpclient
>     
>     Findings:
>     - 32 int shifted by an amount not in range -31..31:
>         The shifts by 32 bits don't actually have any effect on the value
>         (as shown by the tests)
>     - possible null pointer dereference
>     - repeated conditional test
>     - field only ever set to null
>         All other uses of the field were to check if it was null,
>         which it was, so it was removed
>     
>     Other actions:
>     - Upgrade jUnit to version 4
>     - Add PowerMock dependency
> 
> 
> Diffs
> -----
> 
>   pom.xml 9dbd72b 
>   services/console-proxy-rdp/rdpconsole/pom.xml 05585a1 
>   services/console-proxy-rdp/rdpconsole/src/main/java/common/Client.java 688693b 
>   services/console-proxy-rdp/rdpconsole/src/main/java/streamer/ByteBuffer.java 3a718ba 
>   services/console-proxy-rdp/rdpconsole/src/test/java/common/ClientTest.java PRE-CREATION 
>   services/console-proxy-rdp/rdpconsole/src/test/java/rdpclient/MockServerTest.java 17bc4bd 
>   services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferParametricTest.java PRE-CREATION 
>   services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18114/diff/
> 
> 
> Testing
> -------
> 
> Added tests for code that changed and tested in Devcloud
> 
> 
> Thanks,
> 
> Miguel Ferreira
> 
>


Re: Review Request 18114: Fix findbugs findings in cloudstack-service-console-proxy-rdpclient

Posted by Hugo Trippaers <ht...@schubergphilis.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18114/#review34851
-----------------------------------------------------------


Miguel, can you check the last version of your patch versus current master? Anything we missed?

- Hugo Trippaers


On Feb. 14, 2014, 2:48 p.m., Miguel Ferreira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18114/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2014, 2:48 p.m.)
> 
> 
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
>     Fix findbugs findings in cloudstack-service-console-proxy-rdpclient
>     
>     Findings:
>     - 32 int shifted by an amount not in range -31..31:
>         The shifts by 32 bits don't actually have any effect on the value
>         (as shown by the tests)
>     - possible null pointer dereference
>     - repeated conditional test
>     - field only ever set to null
>         All other uses of the field were to check if it was null,
>         which it was, so it was removed
>     
>     Other actions:
>     - Upgrade jUnit to version 4
>     - Add PowerMock dependency
> 
> 
> Diffs
> -----
> 
>   pom.xml 9dbd72b 
>   services/console-proxy-rdp/rdpconsole/pom.xml 05585a1 
>   services/console-proxy-rdp/rdpconsole/src/main/java/common/Client.java 688693b 
>   services/console-proxy-rdp/rdpconsole/src/main/java/streamer/ByteBuffer.java 3a718ba 
>   services/console-proxy-rdp/rdpconsole/src/test/java/common/ClientTest.java PRE-CREATION 
>   services/console-proxy-rdp/rdpconsole/src/test/java/rdpclient/MockServerTest.java 17bc4bd 
>   services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferParametricTest.java PRE-CREATION 
>   services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18114/diff/
> 
> 
> Testing
> -------
> 
> Added tests for code that changed and tested in Devcloud
> 
> 
> Thanks,
> 
> Miguel Ferreira
> 
>


Re: Review Request 18114: Fix findbugs findings in cloudstack-service-console-proxy-rdpclient

Posted by Miguel Ferreira <mi...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18114/#review34990
-----------------------------------------------------------

Ship it!


Ship It!

- Miguel Ferreira


On Feb. 14, 2014, 2:48 p.m., Miguel Ferreira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18114/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2014, 2:48 p.m.)
> 
> 
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
>     Fix findbugs findings in cloudstack-service-console-proxy-rdpclient
>     
>     Findings:
>     - 32 int shifted by an amount not in range -31..31:
>         The shifts by 32 bits don't actually have any effect on the value
>         (as shown by the tests)
>     - possible null pointer dereference
>     - repeated conditional test
>     - field only ever set to null
>         All other uses of the field were to check if it was null,
>         which it was, so it was removed
>     
>     Other actions:
>     - Upgrade jUnit to version 4
>     - Add PowerMock dependency
> 
> 
> Diffs
> -----
> 
>   pom.xml 9dbd72b 
>   services/console-proxy-rdp/rdpconsole/pom.xml 05585a1 
>   services/console-proxy-rdp/rdpconsole/src/main/java/common/Client.java 688693b 
>   services/console-proxy-rdp/rdpconsole/src/main/java/streamer/ByteBuffer.java 3a718ba 
>   services/console-proxy-rdp/rdpconsole/src/test/java/common/ClientTest.java PRE-CREATION 
>   services/console-proxy-rdp/rdpconsole/src/test/java/rdpclient/MockServerTest.java 17bc4bd 
>   services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferParametricTest.java PRE-CREATION 
>   services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18114/diff/
> 
> 
> Testing
> -------
> 
> Added tests for code that changed and tested in Devcloud
> 
> 
> Thanks,
> 
> Miguel Ferreira
> 
>


Re: Review Request 18114: Fix findbugs findings in cloudstack-service-console-proxy-rdpclient

Posted by Miguel Ferreira <mi...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18114/
-----------------------------------------------------------

(Updated Feb. 14, 2014, 2:48 p.m.)


Review request for cloudstack, daan Hoogland and Hugo Trippaers.


Changes
-------

The previous patch was very likely rebased on the wrong brnach and had potentialyl included unintended changes.
This update should fix that.


Repository: cloudstack-git


Description
-------

    Fix findbugs findings in cloudstack-service-console-proxy-rdpclient
    
    Findings:
    - 32 int shifted by an amount not in range -31..31:
        The shifts by 32 bits don't actually have any effect on the value
        (as shown by the tests)
    - possible null pointer dereference
    - repeated conditional test
    - field only ever set to null
        All other uses of the field were to check if it was null,
        which it was, so it was removed
    
    Other actions:
    - Upgrade jUnit to version 4
    - Add PowerMock dependency


Diffs (updated)
-----

  pom.xml 9dbd72b 
  services/console-proxy-rdp/rdpconsole/pom.xml 05585a1 
  services/console-proxy-rdp/rdpconsole/src/main/java/common/Client.java 688693b 
  services/console-proxy-rdp/rdpconsole/src/main/java/streamer/ByteBuffer.java 3a718ba 
  services/console-proxy-rdp/rdpconsole/src/test/java/common/ClientTest.java PRE-CREATION 
  services/console-proxy-rdp/rdpconsole/src/test/java/rdpclient/MockServerTest.java 17bc4bd 
  services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferParametricTest.java PRE-CREATION 
  services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/18114/diff/


Testing
-------

Added tests for code that changed and tested in Devcloud


Thanks,

Miguel Ferreira


Re: Review Request 18114: Fix findbugs findings in cloudstack-service-console-proxy-rdpclient

Posted by Miguel Ferreira <mi...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18114/
-----------------------------------------------------------

(Updated Feb. 14, 2014, 11:46 a.m.)


Review request for cloudstack, daan Hoogland and Hugo Trippaers.


Changes
-------

Fix merge problem and add licens eheader to new files


Repository: cloudstack-git


Description
-------

    Fix findbugs findings in cloudstack-service-console-proxy-rdpclient
    
    Findings:
    - 32 int shifted by an amount not in range -31..31:
        The shifts by 32 bits don't actually have any effect on the value
        (as shown by the tests)
    - possible null pointer dereference
    - repeated conditional test
    - field only ever set to null
        All other uses of the field were to check if it was null,
        which it was, so it was removed
    
    Other actions:
    - Upgrade jUnit to version 4
    - Add PowerMock dependency


Diffs (updated)
-----

  pom.xml 9dbd72b 
  services/console-proxy-rdp/rdpconsole/pom.xml 05585a1 
  services/console-proxy-rdp/rdpconsole/src/main/java/common/Client.java 688693b 
  services/console-proxy-rdp/rdpconsole/src/main/java/streamer/ByteBuffer.java 3a718ba 
  services/console-proxy-rdp/rdpconsole/src/test/java/common/ClientTest.java PRE-CREATION 
  services/console-proxy-rdp/rdpconsole/src/test/java/rdpclient/MockServerTest.java 17bc4bd 
  services/console-proxy-rdp/rdpconsole/src/test/java/streamer/ByteBufferTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/18114/diff/


Testing
-------

Added tests for code that changed and tested in Devcloud


Thanks,

Miguel Ferreira