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

od: fix parsing of hex input ending with E #4983

Merged
merged 1 commit into from
Jun 20, 2023

Conversation

TheDcoder
Copy link
Contributor

This should fix #4981.

This is my first contribution in Rust and I just started learning it today, so pardon me if I missed something, do let me know and I will fix it promptly!

Special thanks to @tertsdiepraam for finding the bug before I even started learning Rust, thus saving me a bunch of time 😄

@sylvestre
Copy link
Contributor

cool, could you please add a test in tests/by-util/test_od.rs ?

@TheDcoder
Copy link
Contributor Author

@sylvestre Ah, I haven't gotten far enough in "the book" to do that yet. Though I did take a look if I can just modify some existing test, but there doesn't seem to be (I might be wrong).

It's worth noting that hex input ending with B didn't have this bug since the initial implementation (459db47) because it already had the safeguard which I added to E.

So IMHO writing a new test is out of scope for both this PR and my technical knowledge in rust 😅

Also I did add a simple assertion to check if 0XE == 14 😄

Let me know what you think, if a new test is needed then I will need more time to study the book and implement it.

@sylvestre
Copy link
Contributor

Adding a test should be trivial
Just copy another example from here:
https:/uutils/coreutils/blob/main/tests/by-util/test_od.rs

@sylvestre sylvestre merged commit 8debc96 into uutils:main Jun 20, 2023
@uutils uutils deleted a comment from github-actions bot Jun 23, 2023
@TheDcoder
Copy link
Contributor Author

@sylvestre Thank you so much once again! Crazy how small things like this end up making such a big difference :)

I'm working on my first rust project right now (it's empty since I haven't committed any code).

@sylvestre
Copy link
Contributor

we have plenty of other good first bugs here if you want

@TheDcoder
Copy link
Contributor Author

I'm still not proficient, so I'm working on my own project to learn the ropes. Will definitely look into fixing some bugs here after that!

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.

od: --skip-bytes argument '0x20E' too large
2 participants