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

Assumed value implies length #40

Closed
mgenereu opened this issue Jul 20, 2014 · 3 comments
Closed

Assumed value implies length #40

mgenereu opened this issue Jul 20, 2014 · 3 comments

Comments

@mgenereu
Copy link

I wrote some test code to learn bindata and I got an error I wasn't expecting.
I had assumed, as the documentation had implied for value (and therefore implied for asserted_value), that the identity string was a fixed length constant of three letters for the purposes of reading, writing, and asserting. Writing went great but reading resulted in a length of zero. I know adding the "read_length" attirbute will fix the issue but it took a trace of the code to figure that out. Doesn't seem intuitive. I would be happy to write a patch if you agree that this is not the expected behavior. Thank you for your review. Here's the code:

require 'bindata'

class TestFormat < BinData::Record
  endian :big
  string :identity, :asserted_value => "xyz"
  uint8 :version, :initial_value => 1
end

test1 = TestFormat.new
test1.version = 65
File.open 'test.bin', 'wb' do |file|
  test1.write( file )
end

test2 = TestFormat.new
File.open 'test.bin', 'rb' do |file|
  test2.read( file )
end

puts test2
@dmendel
Copy link
Owner

dmendel commented Jul 20, 2014

Thanks for the report.

The length should be explicit so the format is understandable when reading the code. If an :asserted_value contains multibyte characters then the length can not be inferred by a casual reader.

A runtime warning if :length is missing would be appropriate. I'll add one in the next release.

dmendel pushed a commit that referenced this issue Jul 20, 2014
@dmendel dmendel closed this as completed Jul 20, 2014
@mgenereu
Copy link
Author

I can't think of a scenario where a static zero is ever an okay length to file read on a fixed string. Setting a runtime warning for :length would be great. I can code if you like. Thank you!

@dmendel
Copy link
Owner

dmendel commented Jul 21, 2014

Note the length should be specified with :read_length not :length. I realised the mistake while reviewing the code this morning. The code is updated in the repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants