Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Array#pack buffer should start from beginning of buffer? #4727

Closed
headius opened this issue Jul 31, 2017 · 0 comments · Fixed by #5953
Closed

Array#pack buffer should start from beginning of buffer? #4727

headius opened this issue Jul 31, 2017 · 0 comments · Fixed by #5953

Comments

@headius
Copy link
Member

headius commented Jul 31, 2017

This is confusing, but we're definitely not passing it. This spec was added recently with the addition of a "buffer" kwarg for Array#pack.

It appears from the result that we are supports to take the "@4" as being from the beginning of the given buffer, leaving the existing four bytes in place and writing the new four non-zero bytes at offset 4. Instead, we append the nulls and the new values.

From test/mri/ruby/test_pack.rb in test_pack_with_buffer:

    buf = String.new(capacity: 100)
    #...
    [0xDEAD_BEEF].pack('N', buffer: buf)
    assert_equal "\xDE\xAD\xBE\xEF", buf

    [0xBABE_F00D].pack('@4N', buffer: buf)
    assert_equal "\xDE\xAD\xBE\xEF\xBA\xBE\xF0\x0D", buf

This is new logic but there's more work needed than I want to put in right now, and this is an unexpected behavior of the buffer.

See #4687 and #4293.

headius added a commit that referenced this issue Jul 31, 2017
See #4727 for remaining issues with how we're handling the buffer.
MariuszCwikla added a commit to MariuszCwikla/jruby that referenced this issue Nov 2, 2019
MariuszCwikla added a commit to MariuszCwikla/jruby that referenced this issue Nov 2, 2019
MariuszCwikla added a commit to MariuszCwikla/jruby that referenced this issue Nov 2, 2019
@enebo enebo added this to the JRuby 9.2.10.0 milestone Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants