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

compound_view::explode() uses bad format string #14577

Closed
tchaikov opened this issue Jul 8, 2023 · 1 comment
Closed

compound_view::explode() uses bad format string #14577

tchaikov opened this issue Jul 8, 2023 · 1 comment
Milestone

Comments

@tchaikov
Copy link
Contributor

tchaikov commented Jul 8, 2023

format("non-zero component divider found ({:d}) mid", format("0x{:02x}", composite::eoc_type(marker) & 0xff)));

format a sstring with {:d} will annoy fmtlib, and it will throw an exception in this case.

tchaikov added a commit to tchaikov/scylladb that referenced this issue Jul 8, 2023
before this change, we format a sstring with "{:d}", fmtlib would throw
`fmt::format_error` at runtime when formatting it. this is not expected.

so, in this change, we just print the int8_t using `seastar::format()`
in a single pass. and with the format specifier of `#02x` instead of
adding the "0x" prefix manually.

Fixes scylladb#14577
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
@tchaikov tchaikov changed the title compound_view::explode() use bad format string compound_view::explode() uses bad format string Jul 8, 2023
@DoronArazii DoronArazii added this to the 5.4 milestone Aug 29, 2023
denesb pushed a commit that referenced this issue Dec 18, 2023
before this change, we format a sstring with "{:d}", fmtlib would throw
`fmt::format_error` at runtime when formatting it. this is not expected.

so, in this change, we just print the int8_t using `seastar::format()`
in a single pass. and with the format specifier of `#02x` instead of
adding the "0x" prefix manually.

Fixes #14577
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes #14578

(cherry picked from commit 27d6ff3)
@denesb
Copy link
Contributor

denesb commented Dec 18, 2023

This is a minor fix for an error-path, but the risk is very low too. Backported to 5.2.

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 a pull request may close this issue.

4 participants