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

Issue 487 #557

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Issue 487 #557

wants to merge 16 commits into from

Conversation

infosechoudini
Copy link

@infosechoudini infosechoudini commented Apr 22, 2022

  • Fixed issue for GH Error when populating a MutableFragmentPacket #487
  • changed fragment packet struct to remove #[length=0] for payload so that users can assign payload to the FragmentPacket
    • based on documentation, it should populate without need of #[length] or #[length_fn]
  • Lastly, due to payload being at end of FragmentPacket, #[length] isn't needed
  • Added a few more #[inline] for speed
  • updated proc_macro2 to 1.0.37, it increases speed as well
  • changed some spacing in decorator.rs
  • added flamegraph profile for benches for pnet_packet/benches
  • added gh_issue_487.rs test to pnet/src/gh_issue_487.rs

@infosechoudini
Copy link
Author

infosechoudini commented Apr 22, 2022

only approve the last workflow, please. i had commented some code in master that i shouldnt have when i renamed master to main.

Rust 112

passes on branch: https://github.com/infosechoudini/libpnet/actions/runs/2208929740

pnet_base/Cargo.toml Outdated Show resolved Hide resolved
pnet_base/src/macaddr.rs Outdated Show resolved Hide resolved
pnet_macros/Cargo.toml Outdated Show resolved Hide resolved
pnet_macros/src/decorator.rs Outdated Show resolved Hide resolved
pnet_macros/src/decorator.rs Outdated Show resolved Hide resolved
pnet_packet/Cargo.toml Outdated Show resolved Hide resolved
pnet_packet/benches/packet_benchmarks.rs Outdated Show resolved Hide resolved
pnet_packet/benches/packet_benchmarks.rs Outdated Show resolved Hide resolved
pnet_packet/benches/perf.rs Show resolved Hide resolved
pnet_packet/src/ipv6.rs Outdated Show resolved Hide resolved
@johanmazelanssi
Copy link

It looks like the PR now has conflicts with the master branch.
A rebase may be required to solve these conflicts.
Thank you very much for helping me with my issue.

@infosechoudini
Copy link
Author

resolved the conflicts

@infosechoudini
Copy link
Author

this can be merged now

pull_request:
branches: [ master ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this,libpnet still uses master.

I'm happy to change this, but it should be separate from this PR.

Copy link
Author

Choose a reason for hiding this comment

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

please change this

@@ -0,0 +1,20 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, thanks for adding these!

accessors = accessors,
name = field.name,
ty_str = ty_str,
ctor = ctor
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume all these whitespace changes are from rustfmt?

@@ -1012,7 +1021,7 @@ fn handle_vec_primitive(
#[allow(trivial_numeric_casts, unused_parens, unused_braces)]
#[cfg_attr(feature = \"clippy\", allow(used_underscore_binding))]
pub fn get_{name}(&self) -> Vec<{inner_ty_str}> {{
use core::cmp::min;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for this change?

I believe this will break libpnet when compiled without the std feature?

{inner_ty_str}Iterable {{
buf: &_self.packet[current_offset..end]
}}.map(|packet| packet.from_packet())
.collect::<Vec<_>>()
.collect::<Vec<_>>()
Copy link
Contributor

Choose a reason for hiding this comment

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

Stray trailing whitespace here.

@@ -1321,7 +1330,7 @@ fn handle_vector_field(
let _self = self;
let current_offset = {co};
let end = min(current_offset + {packet_length}, _self.packet.len());

Copy link
Contributor

Choose a reason for hiding this comment

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

And here.

@@ -383,6 +383,7 @@ fn ipv4_packet_option_test() {
0x03, /* length */
0x10, /* data */];

println!("{:?}", packet);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this was left in by mistake?



#[test]
fn main() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this get picked up when running cargo test?

Copy link
Author

Choose a reason for hiding this comment

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

yes

Copy link
Author

Choose a reason for hiding this comment

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

AFAIK

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.

None yet

3 participants