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

Gilles Sadowski commented on CODEC-264:
---------------------------------------

There are formatting issues in the PR (alignment, tab).  It seems that tabs 
were used in the original file. :-(
I'd suggest to make a PR to fix that first, but I don't know what the regular 
contributors to [Codec] would recommend.  Better ask on the "dev" ML.  Thanks.

In general the PR associated with well-defined issue and a JIRA report should 
come with a _single_ commit whose message is prefixed with the JIRA identifier, 
i.e. in this case:
{noformat}
CODEC-264: Fixed sign extensions.
{noformat}
and the 
[src/changes/changes.xml|https://github.com/apache/commons-codec/blob/master/src/changes/changes.xml]
 file should be updated accordingly.

Similarly, unit tests that ensure that the fix is working should be either 
named according to what they test (e.g. {{testHash64SignExtension}} or 
something) or to the JIRA issue (e.g. {{testCodec264}}).  Of course, you are 
welcome to mention attribution in the commit message:
{noformat}
CODEC-264: Fixed sign extensions.

Thanks to "yonik" for documenting the issue.
{noformat}
and/or in a code comment in the unit tests Java file.

> murmur3.hash64() does not account for unsigned in arguments
> -----------------------------------------------------------
>
>                 Key: CODEC-264
>                 URL: https://issues.apache.org/jira/browse/CODEC-264
>             Project: Commons Codec
>          Issue Type: Bug
>    Affects Versions: 1.13
>            Reporter: Claude Warren
>            Priority: Major
>         Attachments: YonikMurmur3Tests.java
>
>
> The original murmur3_x64_128 code used unsigned int for seed arguments.  
> Using the equivalent bit patterns in the commons codec version does not yield 
> the same results.
> I believe this is because the commons version does not account for sign 
> extension etc.
> Yonic Seeley [~yonik] has explains the issue in his implementation 
> https://github.com/yonik/java_util/blob/master/src/util/hash/MurmurHash3.java
> He provides a test case to show that his code returns the same answers as the 
> original C/C++ code.  I modified that test to call the codec version to show 
> the error.
> I have attached that test case.
> Given that the original code is in the wild I am uncertain how to fix this 
> issue.



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

Reply via email to