Replace Bsnprintf() implementation and add format attributes#1697
Merged
BareosBot merged 23 commits intobareos:masterfrom Jun 3, 2025
Merged
Replace Bsnprintf() implementation and add format attributes#1697BareosBot merged 23 commits intobareos:masterfrom
BareosBot merged 23 commits intobareos:masterfrom
Conversation
sebsura
reviewed
Feb 6, 2024
sebsura
reviewed
Feb 6, 2024
Contributor
|
I set this to no build to reduce the strain on jenkins. As soon as we get it to build locally this should be changed back. |
dba59d4 to
5fb5295
Compare
6774b89 to
bb5cb43
Compare
3eeb351 to
210e60b
Compare
4c5391b to
23e9cee
Compare
076cdb2 to
f8669ce
Compare
b309df4 to
8c3ec2f
Compare
arogge
commented
May 26, 2025
arogge
commented
May 27, 2025
Member
Author
arogge
left a comment
There was a problem hiding this comment.
I looked at it and am mostly happy. I have a few concerns that we may want to address (or maybe not)
- Bareos own type names usually use PascalCase
- quite a few newly introduced casts use C-style casts (ES.49)
- we now have a lot of
format("%s", string), maybe we can find a way to have a safeformat(string)that just skips formatting - I'm not sure I fully grasp why integral constants are
inline constexprand string constants areconstexpr const char[](i.e. without the inline
sebsura
approved these changes
Jun 2, 2025
98ac610 to
ae52417
Compare
Replace the existing code with a simple wrapper around the standard library.
The changes to printf lead to slight differences in some output. This patch adapts the existing test to the new outputs.
We use a macro for this to (possibly) support multiple compliers. Currently only gcc and clang support this feature. On msvc the macro currently just expands to nothing.
5c5f58b to
d5eca98
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR replaces the implementation of Bvsnprintf() by the plain library functions.
We also add the attributes to all printf()-like functions to be able to detect problems at compile-time instead of crashing at runtime. As this produced a lot of problems, -Werror was disabled for format-related things (but should be re-enabled before this PR is merged).
Please check
If you have any questions or problems, please give a comment in the PR.
Helpful documentation and best practices
Checklist for the reviewer of the PR (will be processed by the Bareos team)
Make sure you check/merge the PR using
devtools/pr-toolto have some simple automated checks run and a proper changelog record added.General
Source code quality