Cleanup and unify reading values from sysfs files #1856
Conversation
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
[Why] We do the same thing over and over, just the file handle is different. [How] We can use some inline functions which we can call instead. Since we inline them, the resulting code should be identical. [Note] One could think about templating this, but passing the format specifier as constant to cumbersome. One could also use only one of the functions, and parse all values as int64, but then we might get issues with floats. Signed-off-by: Soeren Grunewald <[email protected]>
[Why] Apparently we have a lot of places where we do similar things, so it would be beneficial that we can use it everywhere. Signed-off-by: Soeren Grunewald <[email protected]>
[Why] While browsing through the code it became clear that we have more spots the function could be used, but with different types. Therefor we switch to a templated approach, to make it easier to use different format specifiers. This also has the benefit that we also inline, no matter what. Signed-off-by: Soeren Grunewald <[email protected]>
[Why] There are some includes in the header, which are not needed by the header itself, but by the source file. Therefor move them into the source file. Also remove the using namespace directive from the header, since it is considered bad practice [1]. Signed-off-by: Soeren Grunewald <[email protected]> -- [1] https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rs-using-directive
[Why] Reading values from file is done basically the same way everywhere. And we now have a helper function which can be used to simplify the code. Signed-off-by: Soeren Grunewald <[email protected]>
[Why] While switching over the CPU readout, it became clear that error propagation wasn't not very nice, since we relied on comparison with the default value. [How] Since we have C++17 we can use std::optional to make the error propagation nicer. Signed-off-by: Soeren Grunewald <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Why
While browsing the code for some info how MangoHud does obtain certain GPU parameters, I notice that there is a lot of duplicated code, which ultimately lead to this PR.
What
This PR introduces new helper function(s) which is then used to replace all the existing occasions where we do
rewind,fflushandfscanfin sequence.As you can see by the number of commits, I went to a few iterations before I finally opted for a template approach, since this should result in basically the same code as before, but in a more generalized way. I also opted to use
std::optional, to make error propagation easier and more transparent.I would love to also move the check, if the
FILE*been valid, into the function. But since this check is also used as a gate-keeper in some cases (seeget_cpu_power_k10temp,get_cpu_power_zenpower, I decided against it for now.Note that I also sorted the include alphabetically and removed unused from the header.