Uninitialized list in error path

[tobiasjakobi]

Hello,

I've just started playing around with xdg-desktop-portal-wlr, and I'm encountering a segfault here: https://github.com/emersion/xdg-desktop-portal-wlr/blob/efcbcb60aa3bc46b0e290d5f6627783fbb534dcd/src/screencast/wlr_screencast.c#L667

My analysis shows that xdpw_wlr_screencopy_init() is never called for me, hence the two lists are not initialized.

Inspecting the coredump confirms this:

gdb) p *ctx
$1 = {state = 0x7ffe0eb63610, pwr_context = 0x13232a0, core = 0x0, output_list = {prev = 0x0, next = 0x0}, registry = 0x0, screencopy_manager = 0x0, 
  xdg_output_manager = 0x0, shm = 0x0, screencast_instances = {prev = 0x0, next = 0x0}}

So this goto statement needs to be fixed: https://github.com/emersion/xdg-desktop-portal-wlr/blob/efcbcb60aa3bc46b0e290d5f6627783fbb534dcd/src/screencast/screencast.c#L477

With best wishes, Tobias

[danshick]

@tobiasjakobi

Thanks for the bug report. We definitely shouldn't be segfaulting, but this is occurring when the app is already in a failed state that will result in an exit.

I don't know when one of us will have time to address these error conditions, but we will gladly accept PRs any time.

If you want to troubleshoot the reason for the error condition, I'd be happy to help. My guess is some issue with pipewire being installed and configured based on the specific goto giving you trouble.

Edit: On second look, this appears to be fairly straight forward to resolve. Just need a second goto that skips the wlr finish routine because we never got to the setup. I'd still want to investigate cleaning up pipewire correctly as part of a proper fix.

[tobiasjakobi]

@tobiasjakobi Edit: On second look, this appears to be fairly straight forward to resolve. Just need a second goto that skips the wlr finish routine because we never got to the setup. I'd still want to investigate cleaning up pipewire correctly as part of a proper fix.

This was also my (naive) conclusion, but I wasn't particularly sure if xdpw_wlr_screencopy_finish() was just the inverse of xdpw_wlr_screencopy_init(), or if finish also did something else (e.g. undo stuff that happens in xdpw_pwr_core_connect()). I'm not yet that familiar with the code :-)

[danshick]

This was also my (naive) conclusion, but I wasn't particularly sure if xdpw_wlr_screencopy_finish() was just the inverse of xdpw_wlr_screencopy_init(), or if finish also did something else (e.g. undo stuff that happens in xdpw_pwr_core_connect()). I'm not yet that familiar with the code :-)

It is a proper inverse (and if it isn't, that's a mistake).