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.

Sign up for GitHub

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

Jump to bottom

GVariant, GError and g_autoptr cleanups #1439

Merged
merged 4 commits into from
Oct 11, 2024
Merged

GVariant, GError and g_autoptr cleanups #1439

merged 4 commits into from
Oct 11, 2024

Conversation

swick
Copy link
Contributor

@swick swick commented Sep 23, 2024

No description provided.

whot
whot approved these changes Oct 4, 2024
View reviewed changes
Copy link
Contributor

@whot whot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the non-floating gvariants patch but it looks correct and the rest too.

src/input-capture.c Show resolved Hide resolved
@swick swick force-pushed the wip/gvariant-ref-sink branch from d940780 to 543178d Compare October 4, 2024 10:56
@swick
Copy link
Contributor Author

swick commented Oct 4, 2024

I'm convinced that using floating references is in general very dangerous and with g_auto there isn't much of a use for them. The only place where not sinking the floating reference immediately is acceptable in my opinion is when it is passed directly as an argument to a function which is clearly documented to sink the reference.

@matthiasclasen
Copy link
Contributor

Pedantic grammar fix for the second commit message:

"... and do in fact re-use it ..."

@matthiasclasen
Copy link
Contributor

I agree with the idea of avoid floating as much as possible

swick added 4 commits October 11, 2024 16:29
@swick
Use g_autoptr for GErrors in more places
cabc3cc
@swick
realtime: Use g_autoptr for non-floating GVariants
7a4fd3e
@swick
Always create an empty vardict instead of passing NULL
3cdc4ab 
In the error case, we will clear the GVariant and then try to emit a
response from a NULL pointer as result. Always create an empty vardict
when something failed instead.

This also clears the error because we continue on in the error case and
do in fact re-use it in one case.
@swick
portals: Work with owned, non-floating GVariants where possible
5ddc28c 
By g_variant_ref_sink'ing floating GVariants as early as possible. This
fixes a few cases where the variant would be leaked, where the floating
ref would be owned by a g_autoptr and makes it easier to see who owns
the reference.

Cases where the floating ref is passed into a function which immediately
sinks the floating ref are left untouched.
@swick swick force-pushed the wip/gvariant-ref-sink branch from 543178d to 5ddc28c Compare October 11, 2024 14:31
@matthiasclasen matthiasclasen added this pull request to the merge queue Oct 11, 2024
Merged via the queue into flatpak:main with commit 8c85331 Oct 11, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whot whot whot approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Triage
Status: Triaged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@swick @matthiasclasen @whot