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

Add checks and specs for writing to inline array and char array fields #756

Merged
merged 3 commits into from Apr 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions ext/ffi_c/StructLayout.c
Expand Up @@ -350,8 +350,13 @@ array_field_put(VALUE self, VALUE pointer, VALUE value)
argv[0] = INT2FIX(f->offset);
argv[1] = value;

rb_funcall2(pointer, rb_intern("put_string"), 2, argv);

if (RSTRING_LEN(value) < array->length) {
rb_funcall2(pointer, rb_intern("put_string"), 2, argv);
} else if (RSTRING_LEN(value) == array->length) {
rb_funcall2(pointer, rb_intern("put_bytes"), 2, argv);
} else {
rb_raise(rb_eIndexError, "String is longer (%ld bytes) than the char array (%d bytes)", RSTRING_LEN(value), array->length);
}
} else {
#ifdef notyet
MemoryOp* op;
Expand Down
56 changes: 56 additions & 0 deletions spec/ffi/struct_spec.rb
@@ -1,3 +1,4 @@
# -*- encoding: utf-8 -*-
#
# This file is part of ruby-ffi.
# For licensing, see LICENSE.SPECS
Expand Down Expand Up @@ -829,6 +830,61 @@ class StructWithArray < FFI::Struct
@s = LibTest::StructWithArray.new(LibTest.struct_make_struct_with_array(0, 1, 2, 3, 4))
expect(@s[:a].to_ptr).to eq(LibTest::struct_field_array(@s.to_ptr))
end

it 'raises when trying to set an array field' do
@s = LibTest::StructWithArray.new(LibTest.struct_make_struct_with_array(0, 1, 2, 3, 4))
expect { @s[:a] = [7, 8, 9, 10, 11] }.to raise_error(NotImplementedError, 'cannot set array field')
end
end

describe FFI::Struct, ' with a char array field' do
module LibTest
class StructWithCharArray < FFI::Struct
layout :before, :int32, :str, [:char, 4], :after, :int32
end
end

before do
@s = LibTest::StructWithCharArray.new
@s.pointer.write_bytes([-1, 1, 2, 0, 255, -2].pack("lC4l"))
end

it 'should read values from memory' do
expect(@s[:before]).to eq(-1)
expect(@s[:str].to_a).to eq([1, 2, 0, -1])
expect(@s[:str].to_s).to eq("\x01\x02".b)
expect(@s[:after]).to eq(-2)
end

it 'should return the number of elements in the array field' do
expect(@s[:str].size).to eq(4)
end

it 'should allow iteration through the array elements' do
@s[:str].each_with_index { |elem, i| expect(elem).to eq([1, 2, 0, -1][i]) }
end

it 'should return the pointer to the array' do
expect(@s[:str].to_ptr).to eq(@s.to_ptr + 4)
end

it 'allows writing a shorter String to the char array' do
@s[:str] = "äc"
expect(@s[:before]).to eq(-1)
expect(@s[:str].to_s).to eq("äc".b)
expect(@s[:after]).to eq(-2)
end

it 'allows writing a String of the same size to the char array' do
@s[:str] = "äcd"
expect(@s[:before]).to eq(-1)
expect(@s[:str].to_s).to eq("äcd".b)
expect(@s[:after]).to eq(-2) # The above should not write to the next field
end

it 'raises when writing a longer String to the char array' do
expect { @s[:str] = "äcde" }.to raise_error(IndexError)
end
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you agree these 2 specs should be the correct behavior?
Current output on MRI:

  1) FFI::Struct  with a char array field allows writing a String of the same size to the char array
     Failure/Error: expect(@s[:after]).to eq(-1) # The above should not write to the next field
     
       expected: -1
            got: -256
     
       (compared using ==)
     # ./spec/ffi/struct_spec.rb:839:in `block (2 levels) in <module:StructSpecs>'

  2) FFI::Struct  with a char array field raises when writing a longer String to the char array
     Failure/Error: expect { @s[:str] = "abcdefg" }.to raise_error(IndexError)
       expected IndexError but nothing was raised
     # ./spec/ffi/struct_spec.rb:843:in `block (2 levels) in <module:StructSpecs>'

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I implemented that behavior in the C extension.

end

describe 'BuggedStruct' do
Expand Down