Cleanup and unify reading values from sysfs files#1856
Open
soerengrunewald wants to merge 6 commits intoflightlessmango:masterfrom
Open
Cleanup and unify reading values from sysfs files#1856soerengrunewald wants to merge 6 commits intoflightlessmango:masterfrom
soerengrunewald wants to merge 6 commits intoflightlessmango:masterfrom
Conversation
[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 <soeren.grunewald@gmx.net>
[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 <soeren.grunewald@gmx.net>
[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 <soeren.grunewald@gmx.net>
[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 <soeren.grunewald@gmx.net> -- [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 <soeren.grunewald@gmx.net>
[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 <soeren.grunewald@gmx.net>
soerengrunewald
force-pushed
the
cleanup/unify-reading-sysfs-files
branch
from
0d4a2e5 to
f42eb86
Compare
|
|
||
|
|
||
| /** Read a single value from sysfs file */ | ||
| static inline int read_as_int(FILE* f, int d = 0) { |
Owner
There was a problem hiding this comment.
You don't end up using these functions so this commit does nothing
Contributor
Author
There was a problem hiding this comment.
Sorry for the late response, somehow I did not receive any notifications or I must have simply overlooked it.
Anyhow, indeed some of the commit are mere intermediate steps, which I left in on purpose.
a) To document the journey, to understand how I ended up with the changes.
b) To ensure one can compile based one every single commit, so git-bisect will work as one would expect.
But if you think it is not necessary, I can squash all or some commits, as you like.
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
2 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.
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.