Skip to content

printf: fix octal escape parsing #7431

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

Merged
merged 2 commits into from
Mar 11, 2025

Conversation

RenjiSann
Copy link
Collaborator

Refit of #7210 by @aryalaadi
I had some trouble rebasing so I just copied the changes manually to a new branch, so your name disappears from the commits. Sorry for that, I'll try to find a way to make you a co-author of these changes
Fixes #7202

@RenjiSann RenjiSann requested a review from cakebaker March 10, 2025 13:46
@aryalaadi
Copy link
Contributor

aryalaadi commented Mar 10, 2025 via email

@RenjiSann RenjiSann force-pushed the printf-fix-octal-escape branch from b966481 to 5326d19 Compare March 10, 2025 13:52
@RenjiSann
Copy link
Collaborator Author

Well it looks like my suggestions are incompatible with other dependencies of escape.rs
Will look further into it soon

Copy link

GNU testsuite comparison:

GNU test failed: tests/misc/echo. tests/misc/echo is passing on 'main'. Maybe you have to rebase?

@cakebaker
Copy link
Contributor

@RenjiSann for adding a co-author: https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors

@RenjiSann RenjiSann force-pushed the printf-fix-octal-escape branch from 5326d19 to 9c6d1fc Compare March 10, 2025 15:35
pub fn parse_escape_code(rest: &mut &[u8]) -> Result<EscapedChar, EscapeError> {
pub fn parse_escape_code(
rest: &mut &[u8],
zero_octal_parsing: Option<OctalParsing>,
Copy link
Contributor

@cakebaker cakebaker Mar 11, 2025

Choose a reason for hiding this comment

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

I'm not sure about the Option here. None currently indicates that the default variant of OctalParsing should be used (and not that there is no OctalParsing). Maybe it makes sense to explicitly mark one of the OctalParsing variants as default and use OctalParsing::default() where None is currently used?

@RenjiSann RenjiSann force-pushed the printf-fix-octal-escape branch from 9c6d1fc to 2cbad60 Compare March 11, 2025 11:42
Co-authored-by: aryal <141743392+aryalaadi@users.noreply.github.com>
@RenjiSann RenjiSann force-pushed the printf-fix-octal-escape branch from 2cbad60 to bbca2ff Compare March 11, 2025 11:48
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)

@cakebaker cakebaker merged commit 9bbe579 into uutils:main Mar 11, 2025
66 checks passed
@cakebaker
Copy link
Contributor

Thanks :)

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.

printf: fails to interpret octal escape sequence with leading 0 in string literal
3 participants