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

Add align_from attribute to PadField #4553

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

douniwan5788
Copy link

Checklist:

  • If you are new to Scapy: I have checked CONTRIBUTING.md (esp. section submitting-pull-requests)
  • I squashed commits belonging together
  • I added unit tests or explained why they are not relevant
  • I executed the regression tests (using cd test && ./run_tests or tox)
  • If the PR is still not finished, please create a Draft Pull Request

fixes #xxx

@douniwan5788
Copy link
Author

@gpotter2 Why would f._align be a tuple?

scapy/scapy/packet.py

Lines 2528 to 2535 in c38a5de

# Add padding optionally
if isinstance(f, PadField):
if isinstance(f._align, tuple):
pad = - cur_len % (f._align[0] * 8)
else:
pad = - cur_len % (f._align * 8)
if pad:
yield "padding", pad

@gpotter2
Copy link
Member

gpotter2 commented Oct 8, 2024

Do you have a use case for this PR? Is there a protocol that needs something like that?

In any case, please add a unit test.

@douniwan5788
Copy link
Author

Do you have a use case for this PR? Is there a protocol that needs something like that?

In any case, please add a unit test.

@gpotter2 Certainly, I will ensure to include the unit test as I work on resolving the bug. Would you kindly explain why f._align is anticipated to be a tuple?

@gpotter2
Copy link
Member

gpotter2 commented Oct 12, 2024

Again,

Is there a protocol that needs something like that?

Regarding

Would you kindly explain why f._align is anticipated to be a tuple?

I honnestly have no idea :) You could try removing it and see if something breaks

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.

2 participants