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

printf: Check precision before writing into stdout. Fix #1879 #6511

Merged
merged 6 commits into from
Jul 1, 2024

Conversation

Kev1n8
Copy link
Contributor

@Kev1n8 Kev1n8 commented Jun 30, 2024

This is my first time opening a pull request. Any suggestions would be greatly appreciated, thank you.

Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

this is an excellent first PR, bravo :)
You have some tests failing and you should run rustfmt on your code

@Kev1n8 Kev1n8 force-pushed the limit-printf-output-size branch from 3e370af to 7dd93c0 Compare June 30, 2024 08:02
@Kev1n8
Copy link
Contributor Author

Kev1n8 commented Jun 30, 2024

@sylvestre Thx for reviewing. I've cargo fmt my code and am currently running tests on my fork of repo.

Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

@Kev1n8
Copy link
Contributor Author

Kev1n8 commented Jun 30, 2024

The code keeps failing one of the CI/CD tests. I didn't change anything relevant so I'm assuming that is not my problem.
Also, it seems that printf is not handling numbers overflowing properly. For instance on a 64-bit system:
cargo run printf %.*d 92233720368547758099999 0
Finished dev profile [unoptimized + debuginfo] target(s) in 0.52s
Running target/debug/coreutils printf '%.*d' 92233720368547758099999 0
printf: '92233720368547758099999': Numerical result out of range
0
Process finished with exit code 1

Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

@Kev1n8
Copy link
Contributor Author

Kev1n8 commented Jun 30, 2024

sry for the messy tree. i made some mistake when merging

@Kev1n8
Copy link
Contributor Author

Kev1n8 commented Jun 30, 2024

I don't see why the fuzz_seq, false test is failing. Could someone offer me some assistance?

@Kev1n8
Copy link
Contributor Author

Kev1n8 commented Jun 30, 2024

@sylvestre, could you kindly help me out? I would greatly appreciate it.

@cakebaker
Copy link
Contributor

The two failing tests in the CI are unrelated to your PR.

@Kev1n8
Copy link
Contributor Author

Kev1n8 commented Jun 30, 2024

The two failing tests in the CI are unrelated to your PR.

Thanks for your reply. I fixed the commit tree for a bit. wonder if it's good enough to merge, and how I should request for review.

@Kev1n8 Kev1n8 force-pushed the limit-printf-output-size branch from 88c829f to 98d26f2 Compare June 30, 2024 16:56
@sylvestre sylvestre force-pushed the limit-printf-output-size branch from 98d26f2 to 198200a Compare June 30, 2024 17:13
@sylvestre sylvestre force-pushed the limit-printf-output-size branch from 198200a to 7dd36d7 Compare June 30, 2024 20:16
@Kev1n8 Kev1n8 changed the title Fix #1879. Check precision before writing into stdout printf: Check precision before writing into stdout. Fix #1879 Jul 1, 2024
@cakebaker cakebaker merged commit e6b6b27 into uutils:main Jul 1, 2024
65 of 68 checks passed
@cakebaker
Copy link
Contributor

Thanks for your PR!

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.

3 participants