NTFS fails on Ruby 2.4

[chessbyte]

See https://travis-ci.org/ManageIQ/manageiq-smartstate/jobs/236485008 and ManageIQ/manageiq-gems-pending#101

• This code is looking for Bignum and fails on ruby 2.4. Bignum and Fixnum were merged to Integer in ruby 2.4. It doesn't look like can just replace Bignum with Integer. I tried.

The failure:

  1) MiqAzureVm Instance methods behaves like MiqVm instance methods .rootTrees should return an array of MiqMountManager objects
     Failure/Error: raise "MIQ(NTFS::DirectoryIndexNode.initialize) Bad child node: #{@child}"

     RuntimeError:
       MIQ(NTFS::DirectoryIndexNode.initialize) Bad child node: 0
     Shared Example Group: "MiqVm instance methods" called from ./spec/miq_vm/miq_azure_vm_image_spec.rb:74
     # ./lib/gems/pending/fs/ntfs/directory_index_node.rb:57:in `initialize'
     # ./lib/gems/pending/fs/ntfs/directory_index_node.rb:25:in `new'
     # ./lib/gems/pending/fs/ntfs/directory_index_node.rb:25:in `block in nodeFactory'
     # ./lib/gems/pending/fs/ntfs/directory_index_node.rb:24:in `loop'
     # ./lib/gems/pending/fs/ntfs/directory_index_node.rb:24:in `nodeFactory'
     # ./lib/gems/pending/fs/ntfs/attrib_index_root.rb:81:in `initialize'
     # ./lib/gems/pending/fs/ntfs/attrib_index_root.rb:53:in `new'
     # ./lib/gems/pending/fs/ntfs/attrib_index_root.rb:53:in `create_from_header'
     # ./lib/gems/pending/fs/ntfs/mft_entry.rb:241:in `createAttribute'
     # ./lib/gems/pending/fs/ntfs/mft_entry.rb:222:in `block in loadAttributes'
     # ./lib/gems/pending/fs/ntfs/mft_entry.rb:221:in `each'
     # ./lib/gems/pending/fs/ntfs/mft_entry.rb:221:in `loadAttributes'
     # ./lib/gems/pending/fs/ntfs/mft_entry.rb:209:in `getAttributes'
     # ./lib/gems/pending/fs/ntfs/mft_entry.rb:204:in `getFirstAttribute'
     # ./lib/gems/pending/fs/ntfs/mft_entry.rb:158:in `indexRoot'
     # ./lib/gems/pending/fs/ntfs/boot_sect.rb:192:in `rootDir'
     # ./lib/gems/pending/fs/MiqFS/modules/NTFS.rb:50:in `fs_init'
     # ./lib/gems/pending/fs/MiqFS/MiqFS.rb:41:in `initialize'
     # ./lib/gems/pending/fs/MiqFS/MiqFS.rb:27:in `new'
     # ./lib/gems/pending/fs/MiqFS/MiqFS.rb:27:in `getFS'
     # ./lib/gems/pending/fs/MiqMountManager.rb:13:in `block in mountVolumes'
     # ./lib/gems/pending/fs/MiqMountManager.rb:11:in `each'
     # ./lib/gems/pending/fs/MiqMountManager.rb:11:in `mountVolumes'
     # ./lib/gems/pending/MiqVm/MiqVm.rb:167:in `rootTrees'
     # ./spec/miq_vm/miq_vm_instance_methods_shared_examples.rb:12:in `block (3 levels) in <top (required)>'

[Fryguy]

@jerryk55 @roliveri Bump 🐢

What exactly is the code in question doing? It looks like it interprets the 8 bytes as a 64-bit unsigned int, and then is (ab)using a Bignum check to verify that the value inside is not actually a 64-bit number but instead a different, smaller, number. I'd say that the code is probably checking for a 32-bit number but if so, then it's a bug, because the threshold for Fixnum/Bignum is not at the 32 bit boundary. For MRI (not even getting into that it's different on JRuby), it's actually at 2**(0.size * 8 - 2) - 1 where 0.size == 8, thus making this 2**62 - 1, which == 4611686018427387903 (more concisely, 2**62 - 1 == 4611686018427387903 is a Fixnum and 2**62 == 4611686018427387904 is a Bignum on a 64-bit compilation of Ruby).

If the goal is to check for this actual threshold, then the code here might just want to compare the value to 2**(0.size * 8 - 2) - 1. If instead, this is actually supposed to be checking for a 32-bit number inside a 64 bit number, then I'd recommend just comparing to 2**32 or mask it off and compare to 0.

[roliveri]

@Fryguy - I removed the offending check in: #10

I believe the check in question was added by the original author of the code, before it was stable. The only way the test could be true is if the data is corrupt. In all other cases, the code assumes the data it reads is valid and uses it. If it's not valid, it will fail down the line. There's no reason the same assumption shouldn't be made here.

I also should add, we have never encountered the exception raised when the condition is true. So, I believe the test is useless.

[jrafanie]

@chessbyte I think this can be closed by #10. We are running on ruby 2.4.1 on travis so, we're good to go here

[jrafanie]

Fryguy closed this a minute ago

Oh @Fryguy beat me to it