Skip to content

Conversation

@kpp
Copy link
Member

@kpp kpp commented Feb 28, 2018

No description provided.

@kpp kpp mentioned this pull request Feb 28, 2018
@kpp kpp force-pushed the dht_bootstrap_info branch from 9b71e62 to da2bb2f Compare February 28, 2018 11:40
@coveralls
Copy link

coveralls commented Feb 28, 2018

Pull Request Test Coverage Report for Build 335

  • 16 of 16 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.924%

Totals Coverage Status
Change from base Build 333: 0.0%
Covered Lines: 6869
Relevant Lines: 7087

💛 - Coveralls

named!(from_bytes<BootstrapInfo>, do_parse!(
tag!(&[0xf0][..]) >>
version: be_u32 >>
motd: rest >> // TODO check motd.len <= 256 (MAX_MOTD_LENGTH)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix todo.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how. The solution so far is:

        tag!(&[0xf0][..]) >>
        version: be_u32 >>
        motd: map!(rest, |bytes| bytes.to_vec() ) >>
        verify!(value!(&motd), |motd: &Vec<_>| motd.len() <= 256) >> //MAX_MOTD_LENGTH
        (BootstrapInfo { version, motd })

Copy link
Member

@kurnevsky kurnevsky Mar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not:

motd: verify!(rest, |motd: &[u8]| motd.len() <= 256) >>
(BootstrapInfo { version, motd.to_vec() })

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@kpp kpp force-pushed the dht_bootstrap_info branch from da2bb2f to 01c50ab Compare March 1, 2018 13:14
@kpp kpp merged commit 1e41101 into master Mar 1, 2018
@kpp kpp deleted the dht_bootstrap_info branch March 1, 2018 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants