Otherwise if some error occurred in e.g. checkAuthorizationSync call, enumerateActions call would also fail (as PolKitAuthority already set error flag internally)
Details
Details
Diff Detail
Diff Detail
- Repository
- R283 KAuth
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
| src/backends/polkit-1/Polkit1Backend.cpp | ||
|---|---|---|
| 220 | This seems a bit weird since we have done nothing with authority here, so how would it have an error? | |
| src/backends/polkit-1/Polkit1Backend.cpp | ||
|---|---|---|
| 220 | Well yes it shouldn't happen now with the two other checks above. This check here just in case (that's why I use qWarning here instead of qCDebug as in two other cases). Also since PolkitAuthoruty is a singleton theoretically some other code using PolkitQt1::Authority methods could trigger such errors. | |
| src/backends/polkit-1/Polkit1Backend.cpp | ||
|---|---|---|
| 220 | Thinking a bit more about it, it can actually happen if enumerateActions call failed. | |
Comment Actions
Have you had these errors happen to you?
How can one reproduce them?
Or is it more of a "this should be the right code but i don't know how to trigger it" case?
Comment Actions
Yes, for me it happens all the time when I try to adjust screen brightness after switching between VT's.
Here is the error that I get with my patch:
KAuth::Polkit1Backend::actionStatus: Encountered error while checking action status, error code: 2 "GDBus.Error:org.freedesktop.PolicyKit1.Error.Failed: Action org.kde.powerdevil.discretegpuhelper.hasdualgpu is not registered"
Sure one has to fix this error in the first place, but it's always a good thing to error-proof the code.
Comment Actions
Did you commit this on purpose without the review being accepted or was it a mistake?
Comment Actions
Yeah, I did it on purpose since:
- It was on review long enough (more than a week), so anyone who wanted to review it had plenty of time to do so.
- There are no open issues.
- The patch itself is very simple: it just checks error status after polkit authority function calls.
- I've been using it localy on 2 different configurations for more than 2 weeks with no issues.
Anyway if you have any comments regarding this patch please feel free to make a postreiew I'll gladly fix any bugs you can find!
Comment Actions
I find this impolite, it was on my queue for reviewing properly, just i have had a very busy week.
But anyway you're the maintainer of kauth now, congratulations :)
Please next time you want to commit something because you feel it's been long enough in the queue i would sincerely appreciate if you say "i'll commit this in a week if noone disagrees", it will give people a chance to say "wait i want to have another look"
Comment Actions
Ah, sorry for the misunderstanding, I simply thought that you didn't care about it anymore :)
Please next time you want to commit something because you feel it's been long enough in the queue i would sincerely appreciate if you say "i'll commit this in a week if noone disagrees", it will give people a chance to say "wait i want to have another look"
That's a very useful advice, I'll definitely follow it, thanks!