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

autocpufreq --version requires sudo now? #530

Closed
niksingh710 opened this issue Jun 26, 2023 · 3 comments · Fixed by #531
Closed

autocpufreq --version requires sudo now? #530

niksingh710 opened this issue Jun 26, 2023 · 3 comments · Fixed by #531

Comments

@niksingh710
Copy link

any command with autocpu-freq requires sudo?

@rootCircle
Copy link
Contributor

rootCircle commented Jun 28, 2023

Looks like a bug from my previous PR #524 . Searching through the code, I got to know that the set_override function is always called irrespective of the force flag is called or not. Check this.

A fix may be either :

  1. To call set_override only when force has some value(non-null). Because, calling it for None values increases extra overhead with no added benefits associated with it.
  2. To prematurely return in set_override function, if override value is None or illegal.

I will try working on the issue and make an appropriate PR ASAP :-)

Till then you can just add an extra (tab) indent on this line. (calling set_override only if force is not None)

rootCircle added a commit to rootCircle/auto-cpufreq that referenced this issue Jun 28, 2023
set_override method is now called only if force flag is invoked.

Here are the specific code changes made:

In auto_cpufreq/core.py:
- Line 96: Removed the root_check call from the set_override function.
           This was intended to be done to comply with code template,
           i.e., to use root_check in the main file.

In bin/auto-cpufreq:
- Line 45: Added a root_check call before calling set_override in main program.
           Also, set_override is only called if force option is invoked, saving
           us from precious extra overheads.

Fixes AdnanHodzic#530
@niksingh710
Copy link
Author

Looks like a bug from my previous PR #524 . Searching through the code, I got to know that the set_override function is always called irrespective of the force flag is called or not. Check this.

A fix may be either :

1. To call `set_override` only when force has some value(non-null). Because, calling it for None values increases extra overhead with no added benefits associated with it.

2. To prematurely return in `set_override` [function](https://github.com/AdnanHodzic/auto-cpufreq/blob/486a9a6dc21b742e1a85db681ac1368a5ef59912/auto_cpufreq/core.py#L98), if `override` value is None or illegal.

I will try working on the issue and make an appropriate PR ASAP :-)

Till then you can just add an extra (tab) indent on this line. (calling set_override only if force is not None)

Premature return will be the preferred option imo as then it will depend on the set_override to handle the values and the core logic have to just call it instead of going over all the checks. Kinda will keep it clean.

More over a check for non-null will also work but not that much what I would like to see.

Btw if you do fix it and create a pr tag me as rn have added sudo's in all of my scripts will refactor them.

@AdnanHodzic opinion will matter more over it I guess.

@rootCircle
Copy link
Contributor

Premature return will be the preferred option imo as then it will depend on the set_override to handle the values and the core logic have to just call it instead of going over all the checks. Kinda will keep it clean.

I guess it might be a better design, but at cost of an extra function call. But going through the codes and patterns, they preferred to include root_check in the main file, and not with the associated function. (eg: remove_daemon etc)

Due, to this I preferred going with 1st approach with a bit of modifications

@AdnanHodzic opinion will matter more over it I guess.

It certainly would, I had created an PR and will see what he had to comment on this.

rootCircle added a commit to rootCircle/auto-cpufreq that referenced this issue Jun 28, 2023
set_override method is now called only if force flag is invoked.

Here are the specific code changes made:

In auto_cpufreq/core.py:
- Line 96: Removed the root_check call from the set_override function.
           This was intended to be done to comply with code template,
           i.e., to use root_check in the main file.

In bin/auto-cpufreq:
- Line 45: Added a root_check call before calling set_override in main program.
           Also, set_override is only called if force option is invoked, saving
           us from precious extra overheads.

Fixes AdnanHodzic#530
AdnanHodzic pushed a commit that referenced this issue Jun 30, 2023
set_override method is now called only if force flag is invoked.

Here are the specific code changes made:

In auto_cpufreq/core.py:
- Line 96: Removed the root_check call from the set_override function.
           This was intended to be done to comply with code template,
           i.e., to use root_check in the main file.

In bin/auto-cpufreq:
- Line 45: Added a root_check call before calling set_override in main program.
           Also, set_override is only called if force option is invoked, saving
           us from precious extra overheads.

Fixes #530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants