Add bit and nibble support

[Fryguy]

Closes #1

@roliveri @chessbyte @jerryk55 Please review.

[miq-bot]

Checked commit Fryguy@3115009 with rubocop 0.27.1
3 files checked, 5 offenses detected

lib/binary_struct.rb

• 🔶 - Line 8, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• 🔶 - Line 9, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• 🔶 - Line 16, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• 🔶 - Line 17, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

spec/binary_struct_spec.rb

• 🔹 - Line 22, Col 121 - Metrics/LineLength - Line is too long. [122/120]

[jerryk55]

Yup. I like this stuff.

[Fryguy]

@jerryk55 If you want to try out the branch locally, before it's merged, and see if it helps you:

• In your binary_struct clone:

  git fetch upstream pull/6/head:pr6
  git checkout pr6
  

• In manageiq/lib/Gemfile, change the binary_struct line and add :path => "/path/to/your/clone/of/binary_struct"

[coveralls]

Coverage decreased (-0.43%) to 99.31% when pulling 3115009 on Fryguy:add_bit_support into 5627972 on ManageIQ:master.

[coveralls]

Coverage decreased (-0.23%) to 99.51% when pulling 3115009 on Fryguy:add_bit_support into 5627972 on ManageIQ:master.

[jerryk55]

So I read your comment in the README.md file and that does explain the issue we have when trying to use less than 8 bits in any one byte (the entire byte is consumed and the remainder tossed). To me - this seems like a bug in Ruby's unpack. Can you explain to me why this isn't a bug there?

[Fryguy]

I'm not sure. It's arguable if it's a bug or intentional, but it's definitely not what you'd expect. That's part of the reason it took me so long to figure out. Our best bet is to add documentation to String#unpack first, and then modify it later with a real code change if it's a bug.

cc @tenderlove - thoughts on upstreaming documentation changes vs code fix changes or both?

[tenderlove]

It's arguable if it's a bug or intentional, but it's definitely not what you'd expect.

I'm not sure if it's intentional. The behavior seems wrong, so it seems like a bug to me. I'll try to track down the origin of the issue and maybe that will give us a clue if we can upstream or not.

[tenderlove]

So here's what I've found. unpack is simply increasing the pointer as it walks the string. Since char is always the smallest, and it's width is 0xFF, the only way it could support sub-char sizes is if we kept a separate number indicating the bit offset, and use that as the "head of the list" rather than the pointer address. We could do sub-sizeof(char) splits, but it seems like it would be a lot of work to change unpack.

Maybe there should be a warning or argument error if the size provided to b is less than sizeof(char)?

Funny enough, you can do this:

irb(main):001:0> "\xFF\x00".unpack('bb')
=> ["1", "0"]

which seems really confusing.

[Fryguy]

I'm assuming that sizeof(char) is typically one, so to determine the bits
you need sizeof(char) * 8 , or is there a machine independent way of
getting the number of bits in a type?

So, at a minimum, if the full feature of supporting sub-zero bits is
rejected or takes too long, I would expect an exception or a warning when
the number is not a multiple of sizeof(char) * 8

b, b1..b7, b9..b15, etc # Exception
b8, b16, etc # ok

Same goes for nibbles, but at sizeof(char) * 2

h, h1, h3, h5, etc # Exception
h2, h4, etc # ok
On May 14, 2015 3:06 PM, "Aaron Patterson" notifications@github.com wrote:

So here's what I've found. unpack is simply increasing the pointer as it
walks the string
https://github.com/ruby/ruby/blob/1709458a20317049e906f1084c43796e1846729b/pack.c#L1353.
Since char is always the smallest, and it's width is 0xFF, the only way
it could support sub-char sizes is if we kept a separate number indicating
the bit offset, and use that as the "head of the list" rather than the
pointer address. We could do sub-sizeof(char) splits, but it seems like
it would be a lot of work to change unpack.

Maybe there should be a warning or argument error if the size provided to
b is less than sizeof(char)?

Funny enough, you can do this:

irb(main):001:0> "\xFF\x00".unpack('bb')
=> ["1", "0"]

which seems really confusing.

—
Reply to this email directly or view it on GitHub
#6 (comment).

[jerryk55]

I like the exception/warning suggestion. I looked for this support in other languages and found something similar in Perl (meaning you couldn't use less than a byte at a time) The issue is you really can't address anything smaller than a byte.

[tenderlove]

I'm assuming that sizeof(char) is typically one, so to determine the bits
you need sizeof(char) * 8 , or is there a machine independent way of
getting the number of bits in a type?

Ya, the code assumes it's 8.

The weird thing is that it specifically takes in to account lengths that aren't divisible by 8 (if I'm reading that correctly), but I guess chooses to ignore the rest? I mean maybe it's so that people can extract the first N bits of each byte? But I don't understand how that would be useful.

[Fryguy]

Going to merge, as there's not more that can be done in this PR. Hopefully, @tenderlove, you can make some headway with upstream. I'd be glad to help out, supply documentation, or whatever, if you could guide me to what you think is best.