Skip to content

Conversation

@sewnie
Copy link

@sewnie sewnie commented Aug 20, 2025

With the toplevel protocol being supported, no NULL check was put in place for it if a compositor does not support it. Here, the check was rewritten to check for the screencast type's required protocol.

Fixes #337

Copy link
Collaborator

@kennylevinsen kennylevinsen left a comment

Choose a reason for hiding this comment

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

How did you trigger this crash?

state->screencast_source_types only includes WINDOW if ext_foreign_toplevel_iamge_capture_source_manager is set, so I'm surprised if a client is asking for it.

I suspect it would be cleaner to handle this in setup_target instead anyway, to ensure we don't show options to the user that we shouldn't have:

type_mask &= ctx->state->screencast_source_types;
if (type_mask == 0) {
    logprint(ERROR, "wlroots: No supported targets specified");
    return;
}

With the toplevel protocol being supported, no NULL check was put
in place for it if a compositor does not support it. Here, the check
was rewritten to check for the screencats type's required protocol.
@sewnie
Copy link
Author

sewnie commented Aug 20, 2025

How did you trigger this crash?

Use a compositor that has ext_foreign_toplevel_image_capture_source_manager as NULL.

state->screencast_source_types only includes WINDOW if ext_foreign_toplevel_iamge_capture_source_manager is set

When I go to select the window, I can select a window.

I suspect it would be cleaner to handle this in setup_target instead anyway, to ensure we don't show options to the user that we shouldn't have:

Good point. Shall i add this fix? you wrote it, so it isn't exactly right of me to steal it :p

@kennylevinsen
Copy link
Collaborator

Feel free to steal it, you discovered the issue anyway. When reviewers share code snippets it's usually with the intent that you can take whatever is useful to complete the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segmentation fault upon window sharing

2 participants