You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Stefania (JIRA)" <ji...@apache.org> on 2016/06/27 03:48:52 UTC

[jira] [Commented] (CASSANDRA-11973) Is MemoryUtil.getShort() supposed to return a sign-extended or non-sign-extended value?

    [ https://issues.apache.org/jira/browse/CASSANDRA-11973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15350391#comment-15350391 ] 

Stefania commented on CASSANDRA-11973:
--------------------------------------

Thank you for the patch, +1. 

From what I could understand, the intention was to return an unsigned short and therefore an int with non-sign extension is correct. There is a comment [here|https://issues.apache.org/jira/browse/CASSANDRA-10579?focusedCommentId=14985202&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14985202] on the ticket that fixed it for the {{UNALIGNED}} case, CASSANDRA-10579, according to which {{MemoryUtil.getShortByByte()}} is already returning an unsigned short, but that's not correct unless we do a {{& 0xFFFF}}.

Therefore, I agree with your patch and, if the tests are OK, I propose to commit it to 2.2+ with a slight variation to avoid duplicating {{& 0xFFFF}}:

||2.2||3.0||trunk||
|[patch|https://github.com/stef1927/cassandra/commits/11973-2.2]|[patch|https://github.com/stef1927/cassandra/commits/11973-3.0]|[patch|https://github.com/stef1927/cassandra/commits/11973]|
|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11973-2.2-testall/]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11973-3.0-testall/]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11973-testall/]|
|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11973-2.2-dtest/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11973-3.0-dtest/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11973-dtest/]|

[~philipthompson]: would it be possible to run the offheap dtests at least with one of these patches?

> Is MemoryUtil.getShort() supposed to return a sign-extended or non-sign-extended value?
> ---------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-11973
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-11973
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>            Reporter: Rei Odaira
>            Assignee: Rei Odaira
>            Priority: Minor
>             Fix For: 2.2.x, 3.0.x, 3.x
>
>         Attachments: 11973-2.2.txt
>
>
> In org.apache.cassandra.utils.memory.MemoryUtil.getShort(), the returned value of unsafe.getShort(address) is bit-wise-AND'ed with 0xffff, while that of getShortByByte(address) is not. This inconsistency results in different returned values when the short integer is negative. Which is preferred behavior? Looking at NativeClustering and NativeCellTest, it seems like non-sign-extension is assumed.
> By the way, is there any reason MemoryUtil.getShort() and MemoryUtil.getShortByByte() return "int", not "short"?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)