FFI::Platform::BYTE_ORDER, FFI::Platform::LITTLE_ENDIAN, FFI::Platform::BIG_ENDIAN #2098

Closed
wants to merge 2 commits into
from

Projects

None yet

3 participants

@DanielVartanov

Original ffi gem provide these constant, some gems rely on them.

@jc00ke jc00ke commented on the diff Dec 13, 2012
kernel/platform/ffi.rb
@@ -257,4 +257,8 @@ def self.mac?
def self.unix?
! windows?
end
+
+ LITTLE_ENDIAN = :little
+ BIG_ENDIAN = :big
+ BYTE_ORDER = Rubinius::ENDIAN == :little ? LITTLE_ENDIAN : BIG_ENDIAN
@jc00ke
jc00ke Dec 13, 2012 Rubinius member

This is kinda cryptic. I'd prefer to see the following.

if Rubinius::ENDIAN == :little # should this just be LITTLE_ENDIAN since it was just defined?
  BYTE_ORDER = LITTLE_ENDIAN
else
  BYTE_ORDER = BIG_ENDIAN
end
@DanielVartanov
DanielVartanov Dec 13, 2012

Well, if ternary operator looks cryptic, there is no problem to change it, I'll update.

should this just be LITTLE_ENDIAN since it was just defined?

It's complicated. The API off ffi assumes only that BYTE_ORDER constant will be equal to LITTLE_ENDIAN constant [or another one], without specifying a value of those constants.

In ffi, LITTLE_ENDIAN is an integer value (look here) and can be changed. I had to assign some value and I chose :little, it could be anything actually.

In Rubinius, Rubinius::ENDIAN constant is assigned here. Therefore, Rubinius::ENDIAN has nothing to do with FFI::Platform::LITTLE_ENDIAN. I'd call it dangerous if we would rely on equality of these constants just because the same symbol is assigned to both of them in two completely different parts of codebase.

What do you think?

@jc00ke jc00ke commented on the diff Dec 13, 2012
spec/ruby/optional/ffi/platform_spec.rb
@@ -125,3 +125,17 @@
end
end
end
+
+describe "FFI::Platform::BYTE_ORDER" do
+ little_endian do
+ it "should be equal to LITTLE_ENDIAN" do
@jc00ke
jc00ke Dec 13, 2012 Rubinius member

We don't use the word should in descriptions.

it "equals LITTLE_ENDIAN" do
   # ...
end
@DanielVartanov
DanielVartanov Dec 13, 2012

Yes, right, I made the same mistake twice. Thank you for noticing.

@brixen
Member
brixen commented Dec 13, 2012

Please hold off on more FFI PRs. I'm going to work on getting the ffi gem to run on Rubinius.

@brixen
Member
brixen commented Jan 9, 2013

We now bundle a 1.2.0 version of the FFI gem, however, there is a bug in RVM that omits the pre-installed gems. So if using RVM, use the recently released 1.3.0 FFI gem which builds correctly on Rubinius.

@brixen brixen closed this Jan 9, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment