Skip to content

Printf: Fix printf hex alternate zero#5811

Merged
tertsdiepraam merged 3 commits intouutils:mainfrom
spineki:fix-printf-hex-alternate-zero
Jan 17, 2024
Merged

Printf: Fix printf hex alternate zero#5811
tertsdiepraam merged 3 commits intouutils:mainfrom
spineki:fix-printf-hex-alternate-zero

Conversation

@spineki
Copy link
Copy Markdown
Contributor

@spineki spineki commented Jan 8, 2024

Summary

  • Fixed case where giving only 0 would crash printf due to an attempt to read it as an octal
  • Fixed wrong prefix for lower and upper hex alternative format. Now, the "0x" is not shown when the value is 0.

Code

GNU:
> printf %#x 0
0

uutils: fails
now shows 0

Fixed an

Related issues:

original : #5810
part of other changes (PR): #5794
part of other changes: #5709
octal similar problem: #5807

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 8, 2024

GNU testsuite comparison:

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

@samueltardieu
Copy link
Copy Markdown
Contributor

I would prefer this PR not to be merged as-is if the larger #5783 PR is merged, as it rewrites the parser used in printf and it doesn't have of the single 0 interpreted as octal already.

@tertsdiepraam
Copy link
Copy Markdown
Collaborator

@samueltardieu I agree. For clarity, you're suggesting to only keep the 0x fix from this, right?

@samueltardieu
Copy link
Copy Markdown
Contributor

@samueltardieu I agree. For clarity, you're suggesting to only keep the 0x fix from this, right?

Yes, and the associated tests.

@spineki
Copy link
Copy Markdown
Contributor Author

spineki commented Jan 9, 2024

I did the octal fix in the first place because printf would fail before reaching the 0x-prefix fix.
Thus, amongst the four new tests, those named xxxx_0 will fail without the octal fix, even if the 0x prefix fix is kept.

It means that if you want to keep a subset of this PR without the octal fix, you can

  • keep tests on lower hex and upper hex (not the 0 variant), but the prefix fix for 0 will not be tested.
  • keep the 4 tests, but the xxx_0 tests variants will fail.

@tertsdiepraam
Copy link
Copy Markdown
Collaborator

Maybe we could try to merge @samueltardieu's PR first and then merge this. That should make everything alright, shouldn't it?

@tertsdiepraam tertsdiepraam force-pushed the fix-printf-hex-alternate-zero branch from 790bfa2 to 0648321 Compare January 10, 2024 15:56
@tertsdiepraam
Copy link
Copy Markdown
Collaborator

I've rebased this on top of main after merging #5783.

@github-actions
Copy link
Copy Markdown

GNU testsuite comparison:

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

@tertsdiepraam tertsdiepraam merged commit 3bd9f0e into uutils:main Jan 17, 2024
@spineki spineki deleted the fix-printf-hex-alternate-zero branch January 17, 2024 17:11
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.

4 participants