move toward option print/parse parity #50

Merged
merged 1 commit into from Apr 1, 2017

Conversation

Projects
None yet
2 participants
@yomimono
Member

yomimono commented Mar 31, 2017

Without this patch, buf_of_pkt will emit options that pkt_of_buf discards. This patch is an attempt to use the existing parser functions to implement the options which buf_of_pkt will output at user request.

@haesbaert

Reads good to me except the min_len which is incorrect.

lib/dhcp_wire.ml
+ | 86 -> take (Nds_tree_name (get_string ()))
+ | 87 -> take (Nds_context (get_string ()))
+ | 88 -> take (Bcmcs_controller_domain_name_list (get_string ()))
+ | 89 -> take (Bcmcs_controller_ipv4_addrs (get_ip_list ~min_len:1 ()))

This comment has been minimized.

@haesbaert

haesbaert Mar 31, 2017

Member

min_len in this function is not List.length, it is the minimum size of the object in bytes, so the correct should be the default min_len:4, which is one ip.

@haesbaert

haesbaert Mar 31, 2017

Member

min_len in this function is not List.length, it is the minimum size of the object in bytes, so the correct should be the default min_len:4, which is one ip.

lib/dhcp_wire.ml
+ | 89 -> take (Bcmcs_controller_ipv4_addrs (get_ip_list ~min_len:1 ()))
+ | 90 -> take (Authentication (get_string ()))
+ | 91 -> take (Client_last_transaction_time (get_32 ()))
+ | 92 -> take (Associated_ips (get_ip_list ~min_len:1 ()))

This comment has been minimized.

@haesbaert

haesbaert Mar 31, 2017

Member

Same here

@haesbaert

haesbaert Mar 31, 2017

Member

Same here

This comment has been minimized.

@yomimono

yomimono Mar 31, 2017

Member

Thanks! min_len is correctly specified in the default for get_ip_list, so I'll just remove it -- not sure what I was thinking here.

@yomimono

yomimono Mar 31, 2017

Member

Thanks! min_len is correctly specified in the default for get_ip_list, so I'll just remove it -- not sure what I was thinking here.

@haesbaert haesbaert merged commit 6c15b6c into mirage:master Apr 1, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stedolan stedolan referenced this pull request in stedolan/crowbar May 24, 2017

Open

Bugs found with Crowbar #2

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