Unreference PolkitAuthorizationResult and PolkitAuthority structs if needed

[Cropi]

1. If polkit_authority_get_sync() fails, then it will return null, so there is no need to free it with g_object_unref().
2. The same goes for polkit_authority_check_authorization_sync(). The documentation says the following: "A PolkitAuthorizationResult is returned or NULL if error is set. Free with g_object_unref()."

I came across this issue when SElinux did not allow usbguard-dbus to perform the mentioned actions. Thus, usbguard tried to free memory, which was not even allocated.

[1] https://www.freedesktop.org/software/polkit/docs/latest/PolkitAuthority.html

[hartwork]

Hi @Cropi ,

for review of this pull request I asked myself two questions:

• Can function polkit_authority_get_sync return NULL?
  I found the answer in file polkit-v.121/src/polkit/polkitauthority.c: Yes, it can.

• Can function g_object_unref be called with NULL, safely?
  After some digging, the answer is: No, it cannot.

With those two answers I agree that this pull request is moving the code into the right direction and fixes a real problem 👍 . I would suggest to also drop the second || error from ! authority || error for both consistency and readability, and then I'd consider it "perfect". What do you think?

Best, Sebastian

[hartwork]

@Cropi PS: For the CI failure with Ubuntu 21.10, I believe the main point was to cover the most recent Ubuntu, more recent than GitHub CI's ubuntu-latest, so maybe we should migrate to Ubuntu 22.04 there, ideally in a dedicated pull request. I have prepared pull request #552 for that now and we could rebase #551 on top of that, after #552 is merged, if you like. What do you think?

[Cropi]

Hi @Cropi ,

for review of this pull request I asked myself two questions:

* Can function `polkit_authority_get_sync` return `NULL`?
  I found the answer in file `polkit-v.121/src/polkit/polkitauthority.c`: Yes, it can.

* Can function `g_object_unref` be called with `NULL`, safely?
  After some digging, the answer is: No, it cannot.

With those two answers I agree that this pull request is moving the code into the right direction and fixes a real problem +1 . I would suggest to also drop the second || error from ! authority || error for both consistency and readability, and then I'd consider it "perfect". What do you think?

Best, Sebastian

Thanks for the review. I definitely agree that we should change the authority part as well to ensure better consistency.

[hartwork]

@Cropi with PRs #548, #551, #552 all merged now — how do you feel about cutting a release 1.1.2 off master?

[Cropi]

@Cropi with PRs #548, #551, #552 all merged now — how do you feel about cutting a release 1.1.2 off master?

I am not against it but usually @radosroka is the one who creates the releases.

[hartwork]

@radosroka any chance or objections?

[radosroka]

@radosroka any chance or objections?

I can create release later today.

[hartwork]

@radosroka cool! Thank you!

[radosroka]

@radosroka cool! Thank you!

Release is out there:

https://github.com/USBGuard/usbguard/releases/tag/usbguard-1.1.2

[hartwork]

@radosroka thank you! Bumped to 1.1.2 in Gentoo as well now.