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
Codechange: use std::string exclusively for settings #9128
Merged
Merged
Conversation
This file contains 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
rubidium42
force-pushed
the
string_settings
branch
from
April 28, 2021 16:30
c17ad3c
to
ccc5883
Compare
frosch123
reviewed
Apr 28, 2021
More general: You use std::string::length() everywhere.
|
rubidium42
force-pushed
the
string_settings
branch
3 times, most recently
from
April 29, 2021 18:41
bd012d6
to
e407e10
Compare
rubidium42
force-pushed
the
string_settings
branch
from
May 1, 2021 17:24
e407e10
to
117c7b6
Compare
rubidium42
commented
May 1, 2021
rubidium42
force-pushed
the
string_settings
branch
7 times, most recently
from
May 6, 2021 21:23
d2ce829
to
31b2713
Compare
rubidium42
force-pushed
the
string_settings
branch
2 times, most recently
from
May 12, 2021 15:01
b3f5485
to
24eda79
Compare
TrueBrain
reviewed
May 13, 2021
rubidium42
force-pushed
the
string_settings
branch
from
May 13, 2021 10:15
24eda79
to
218f16a
Compare
frosch123
reviewed
May 13, 2021
rubidium42
force-pushed
the
string_settings
branch
from
May 13, 2021 20:47
218f16a
to
66b81ae
Compare
frosch123
approved these changes
May 13, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation / Problem
Some (string) settings you want to validate before the setting is written as to prevent the user from entering something invalid. For example an empty client name. Now the (string) validation can only happen in the post set update, so there is no way to revert anymore. To implement that it would be nice not to have to worry about three types of strings coming from the settings: a C-string buffer, a heap allocated C-string and std::string.
For the media things a start has already been made with std::string, so this is just the continuation of that change.
Description
Everywhere where a C-string buffer or heap allocated C-string setting was used this is converted to std::string, along with some of the related logic for handling them. Arguably things might be pushed even further down the stacks, but that would balloon this PR so usually the bare minimum has been done.
Limitations
Empty digit separators are not possible anymore as they now fall back to the language's default.
The overall diff could be smaller if I chose to go back to SDT_STR etc, instead of SDT_SSTR. However, I chose not to do that so other people with patches get a compile time failure instead of a weird crash when it interprets char buffers as std::string or so.
Some variables, those that do not go over the network, I have generally not given an artificial limit as it's not really important how long e.g. a hostname is. It might now truncate it at a later stage in the application, but when the underlying code has been updated to fully use std::string that truncation problem should have gone away.
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.