ckruse
commented
7 years ago Hu, this is surprising for me. I have to set up a testing environment to debug this.
Closed voxik closed 7 years ago
ckruse
commented
7 years ago Hu, this is surprising for me. I have to set up a testing environment to debug this.
voxik
commented
7 years ago So I was going to submit this patch:
diff --git a/lib/cfpropertylist/rbBinaryCFPropertyList.rb b/lib/cfpropertylist/rbBinaryCFPropertyList.rb
index 15301c3..9015aa7 100644
--- a/lib/cfpropertylist/rbBinaryCFPropertyList.rb
+++ b/lib/cfpropertylist/rbBinaryCFPropertyList.rb
@@ -166,9 +172,9 @@ module CFPropertyList
when 1 # 2 byte float? must be an error
raise CFFormatError.new("got #{length+1} byte float, must be an error!")
when 2 then
- buff.reverse.unpack("f")[0]
+ buff.reverse.unpack("e")[0]
when 3 then
- buff.reverse.unpack("d")[0]
+ buff.reverse.unpack("E")[0]
else
fail "unexpected length: #{length}"
end
@@ -190,9 +196,9 @@ module CFPropertyList
when 1 then # 2 byte CFDate is an error
raise CFFormatError.new("#{length+1} byte CFDate, error")
when 2 then
- buff.reverse.unpack("f")[0]
+ buff.reverse.unpack("e")[0]
when 3 then
- buff.reverse.unpack("d")[0]
+ buff.reverse.unpack("E")[0]
end,
CFDate::TIMESTAMP_APPLE
)
@@ -498,7 +504,7 @@ module CFPropertyList
# Codes a real value to binary format
def real_to_binary(val)
- Binary.type_bytes(0b0010,3) << [val].pack("d").reverse
+ Binary.type_bytes(0b0010,3) << [val].pack("E").reverse
end
# Converts a numeric value to binary and adds it to the object table
@@ -545,7 +551,7 @@ module CFPropertyList
val = val.getutc.to_f - CFDate::DATE_DIFF_APPLE_UNIX # CFDate is a real, number of seconds since 01/01/2001 00:00:00 GMT
@object_table[@written_object_count] =
- (Binary.type_bytes(0b0011, 3) << [val].pack("d").reverse)
+ (Binary.type_bytes(0b0011, 3) << [val].pack("E").reverse)
@written_object_count += 1
@written_object_count - 1Where I am replacing the "native format" specifiers for "little-endian" and hence the test suite passes everywhere.
I thought I should also check what the format really should be, but checking Wikipedia 1 and sources 2 it seems this patch is wrong. But not only the patch, but also all the ".plist" files used in tests!!!
voxik
commented
7 years ago Let me put this explicitly, the .plist files used in tests seems to be little endian, where they should be big endian according to specs. It did not pop up earlier, since the "native format" was used ... I just can't explain that nobody noticed yet, but probably nobody is using the binary .plist files?
voxik
commented
7 years ago This should be the patch following the specifications but failing everywhere due to wrong .plist files:
diff --git a/lib/cfpropertylist/rbBinaryCFPropertyList.rb b/lib/cfpropertylist/rbBinaryCFPropertyList.rb
index 15301c3..9015aa7 100644
--- a/lib/cfpropertylist/rbBinaryCFPropertyList.rb
+++ b/lib/cfpropertylist/rbBinaryCFPropertyList.rb
@@ -166,9 +172,9 @@ module CFPropertyList
when 1 # 2 byte float? must be an error
raise CFFormatError.new("got #{length+1} byte float, must be an error!")
when 2 then
- buff.reverse.unpack("f")[0]
+ buff.reverse.unpack("g")[0]
when 3 then
- buff.reverse.unpack("d")[0]
+ buff.reverse.unpack("G")[0]
else
fail "unexpected length: #{length}"
end
@@ -190,9 +196,9 @@ module CFPropertyList
when 1 then # 2 byte CFDate is an error
raise CFFormatError.new("#{length+1} byte CFDate, error")
when 2 then
- buff.reverse.unpack("f")[0]
+ buff.reverse.unpack("g")[0]
when 3 then
- buff.reverse.unpack("d")[0]
+ buff.reverse.unpack("G")[0]
end,
CFDate::TIMESTAMP_APPLE
)
@@ -498,7 +504,7 @@ module CFPropertyList
# Codes a real value to binary format
def real_to_binary(val)
- Binary.type_bytes(0b0010,3) << [val].pack("d").reverse
+ Binary.type_bytes(0b0010,3) << [val].pack("G").reverse
end
# Converts a numeric value to binary and adds it to the object table
@@ -545,7 +551,7 @@ module CFPropertyList
val = val.getutc.to_f - CFDate::DATE_DIFF_APPLE_UNIX # CFDate is a real, number of seconds since 01/01/2001 00:00:00 GMT
@object_table[@written_object_count] =
- (Binary.type_bytes(0b0011, 3) << [val].pack("d").reverse)
+ (Binary.type_bytes(0b0011, 3) << [val].pack("G").reverse)
@written_object_count += 1
@written_object_count - 1I can send PR with the patch, but not sure how to check the .plist files, since I am not Mac user ...
ckruse
commented
7 years ago All PList files have been tested to work with the macOS plist editor (and I just checked again). I'm not sure if big endian is really the right format, although I see your arguments; maybe I should do further investigation. Yes, please create a pull request!
voxik
commented
7 years ago So I'll create two PR, on LE and the other BE and you can pick the right one ;)
ckruse
commented
7 years ago Thank you very much!
Via Mobile, deshalb kurz. Nicht unhöflich gemeint!
Am 24.11.2016 um 12:30 schrieb Vít Ondruch notifications@github.com:
So I'll create two PR, on LE and the other BE and you can pick the right one ;)
— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.
ckruse
commented
7 years ago Little endian shows the right results here…
ckruse
commented
7 years ago Yeah, it's definitely the fix-le branch…
ckruse
commented
7 years ago Thank you very much for your patch! I merged the pull request and closed the other and I just released a new version
Trying to run the test suite of CFProperty list on PPC64, which is BigEndian platfor, fails with following errors: