primitive wrapper types for u16 & u32 #97

zeeshanlakhani · 2020-05-09T04:44:18Z

Description

"new" types to ensure proper conversion of packet data to/from network byte order.

Includes:

impls bit and/or/xor/*assign and not (!) for types
includes updates throughout the packets module to use these types
added flag tests

Notes:

currently includes redo neighbor discovery option parsing API #94 and will be rebased accordingly after that one is reviewed/merged.

Checklist

Associated tests
Associated documentation

codecov · 2020-05-09T04:55:01Z

Codecov Report

Merging #97 into master will increase coverage by 0.28%.
The diff coverage is 86.88%.

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   67.07%   67.36%   +0.28%     
==========================================
  Files          62       63       +1     
  Lines        5260     5331      +71     
==========================================
+ Hits         3528     3591      +63     
- Misses       1732     1740       +8

Impacted Files	Coverage Δ
core/src/net/cidr/v4.rs	`66.66% <ø> (ø)`
core/src/net/cidr/v6.rs	`66.66% <ø> (ø)`
core/src/packets/checksum.rs	`92.30% <ø> (ø)`
core/src/packets/icmp/v6/ndp/router_solicit.rs	`54.76% <ø> (ø)`
core/src/packets/icmp/v6/time_exceeded.rs	`68.65% <ø> (ø)`
core/src/packets/ip/mod.rs	`47.36% <ø> (ø)`
core/src/packets/mod.rs	`96.05% <ø> (ø)`
core/src/runtime/mod.rs	`0.00% <ø> (ø)`
core/src/testils/criterion.rs	`0.00% <ø> (ø)`
core/src/packets/ethernet.rs	`80.13% <33.33%> (-1.08%)`	⬇️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b0ab55...c5f655c. Read the comment docs.

drunkirishcoder

really love the compiler safety this change introduces.

one more comment, fix the commit message. it's too long. see our contributing guideline. lol

core/src/packets/mod.rs

core/src/packets/types.rs

drunkirishcoder · 2020-05-09T15:38:29Z

core/src/packets/types.rs

+use std::fmt;
+use std::ops;
+
+/// Big-endian wrapper type for `u16`.


The 16-bit unsigned integer in big-endian order.

core/src/packets/types.rs

drunkirishcoder · 2020-05-09T15:43:56Z

core/src/packets/types.rs

+
+impl u16be {
+    /// U16 Big-Endian 0
+    pub const BE0: u16be = u16be(0);


actually we can call this MIN, and we can copy the doc over as well. why not.

The smallest value that can be represented by this integer type.

core/src/packets/ethernet.rs

core/src/packets/icmp/v6/ndp/redirect.rs

core/src/packets/icmp/v6/too_big.rs

core/src/packets/ip/v4.rs

core/src/packets/ip/v6/fragment.rs

…packet data to/from network byte order - impls bit and/or/xor/*assign and not (!) for types - includes updates throughout the packets module to use these types - added flag tests

drunkirishcoder

A few small comments. once addressed. 👍

drunkirishcoder · 2020-05-11T16:29:05Z

core/src/packets/types.rs

+pub struct u16be(pub u16);
+
+impl u16be {
+    /// The 16-bit unsigned integer in big-endian order.


I think you made this change in the wrong place. this is for the main type description above. description here should be,

The smallest value that can be represented by this integer type.

drunkirishcoder · 2020-05-11T16:37:52Z

core/src/testils/criterion.rs

@@ -34,10 +34,10 @@ use std::time::{Duration, Instant};
 pub trait BencherExt {
    /// Times a `routine` with an input generated via a `proptest strategy`
    /// batch of input, and then times the iteration of the benchmark over the
-    /// input. See [`BatchSize`] for details on choosing the batch size. The
+    /// input. See [BatchSize] for details on choosing the batch size. The


this is actually a rust type.

diff lib, but ok, yeah.

drunkirishcoder · 2020-05-11T16:39:30Z

core/src/testils/proptest/mod.rs

@@ -16,12 +16,12 @@
 * SPDX-License-Identifier: Apache-2.0
 */

-//! Defines extended [`proptest`] [`Arbitrary`] traits and [`Strategies`] for
+//! Defines extended [proptest] [Arbitrary] traits and [Strategies] for


Arbitrary and Strategy are both traits. Maybe change Strategies to singular? Something like

Defines extended proptest Arbitrary and Strategy traits...

primitive wrapper types for u16 & u32 to ensure proper conversion of packet data to/from network byte order. Includes: - impls bit and/or/xor/*assign and not (!) for types - includes updates throughout the packets module to use these types - additional flag tests

zeeshanlakhani requested a review from drunkirishcoder May 9, 2020 04:44

zeeshanlakhani changed the title ~~Zl/be types~~ be wrapper types May 9, 2020

zeeshanlakhani changed the title ~~be wrapper types~~ big-endian wrapper types and conversions May 9, 2020

drunkirishcoder requested changes May 9, 2020

View reviewed changes

zeeshanlakhani force-pushed the zl/be-types branch from f44ba65 to 2d57ab1 Compare May 9, 2020 17:59

zeeshanlakhani changed the title ~~big-endian wrapper types and conversions~~ primitive wrapper types for u16 & u32 May 9, 2020

zeeshanlakhani force-pushed the zl/be-types branch from 2d57ab1 to 6699d6c Compare May 11, 2020 03:02

Zeeshan Lakhani added 2 commits May 11, 2020 12:06

primitive wrapper types for u16 & u32 to ensure proper conversion of …

cb34a0d

…packet data to/from network byte order - impls bit and/or/xor/*assign and not (!) for types - includes updates throughout the packets module to use these types - added flag tests

clean-up comments

c5cdf8f

zeeshanlakhani force-pushed the zl/be-types branch from 6699d6c to c5cdf8f Compare May 11, 2020 16:09

drunkirishcoder approved these changes May 11, 2020

View reviewed changes

address comments

c5f655c

zeeshanlakhani merged commit 3e085e8 into master May 11, 2020

zeeshanlakhani deleted the zl/be-types branch May 11, 2020 18:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

primitive wrapper types for u16 & u32 #97

primitive wrapper types for u16 & u32 #97

zeeshanlakhani commented May 9, 2020 •

edited

Loading

codecov bot commented May 9, 2020 •

edited

Loading

drunkirishcoder left a comment

drunkirishcoder May 9, 2020

drunkirishcoder May 9, 2020

drunkirishcoder left a comment

drunkirishcoder May 11, 2020

zeeshanlakhani May 11, 2020

drunkirishcoder May 11, 2020

zeeshanlakhani May 11, 2020

drunkirishcoder May 11, 2020

primitive wrapper types for u16 & u32 #97

primitive wrapper types for u16 & u32 #97

Conversation

zeeshanlakhani commented May 9, 2020 • edited Loading

Description

Checklist

codecov bot commented May 9, 2020 • edited Loading

Codecov Report

drunkirishcoder left a comment

Choose a reason for hiding this comment

drunkirishcoder May 9, 2020

Choose a reason for hiding this comment

drunkirishcoder May 9, 2020

Choose a reason for hiding this comment

drunkirishcoder left a comment

Choose a reason for hiding this comment

drunkirishcoder May 11, 2020

Choose a reason for hiding this comment

zeeshanlakhani May 11, 2020

Choose a reason for hiding this comment

drunkirishcoder May 11, 2020

Choose a reason for hiding this comment

zeeshanlakhani May 11, 2020

Choose a reason for hiding this comment

drunkirishcoder May 11, 2020

Choose a reason for hiding this comment

zeeshanlakhani commented May 9, 2020 •

edited

Loading

codecov bot commented May 9, 2020 •

edited

Loading