[jira] [Commented] (ARROW-4018) [C++] RLE decoder may not big-endian compatible

[Antoine Pitrou (Jira)]

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

Antoine Pitrou commented on ARROW-4018:
---------------------------------------

> Are we actually clean in terms of endianness in other places?

Presumably no, because we're reinterpreting array bytes as larger types such as 
int64_t etc. And we're also serializing those bytes directly to disk or wire.

>  it sounds strange to be slicing a long like coverity describes have you 
> looked to see if this is intended?

I think it's just a dirty implementation shortcut. Instead of doing:
{code:cpp}
    bool result =
        bit_reader_.GetAligned<T>(static_cast<int>(BitUtil::CeilDiv(bit_width_, 
8)),
                                  reinterpret_cast<T*>(&current_value_));
{code}
The code could presumably be written as:
{code:cpp}
    T value;
    bool result =
        bit_reader_.GetAligned<T>(static_cast<int>(BitUtil::CeilDiv(bit_width_, 
8)), &value);
    current_value_ = static_cast<uint64_t>(value);
{code}



> [C++] RLE decoder may not big-endian compatible
> -----------------------------------------------
>
>                 Key: ARROW-4018
>                 URL: https://issues.apache.org/jira/browse/ARROW-4018
>             Project: Apache Arrow
>          Issue Type: Bug
>          Components: C++
>    Affects Versions: 0.11.1
>            Reporter: Antoine Pitrou
>            Priority: Major
>             Fix For: 1.0.0
>
>
> This issue was found by Coverity. The {{RleDecoder::NextCounts}} method has 
> the following code to fetch the repeated literal in repeated runs:
> {code:c++}
>     bool result =
>         
> bit_reader_.GetAligned<T>(static_cast<int>(BitUtil::CeilDiv(bit_width_, 8)),
>                                   reinterpret_cast<T*>(&current_value_));
> {code}
> Coverity says this:
> bq. Pointer "&this->current_value_" points to an object whose effective type 
> is "unsigned long long" (64 bits, unsigned) but is dereferenced as a narrower 
> "unsigned int" (32 bits, unsigned). This may lead to unexpected results 
> depending on machine endianness.
> bq. 
> In addition, it's not obvious whether {{current_value_}} also needs 
> byte-swapping (presumably, at least in the Parquet file format, it's supposed 
> to be stored in little-endian format in the RLE bitstream).



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