Skip to content

Adds lookup to the XDG_DATA_DIRS for the wayland session files - #123

Draft
avpcretois wants to merge 2 commits into
kavau:mainfrom
avpcretois:xdg-data-dirs
Draft

Adds lookup to the XDG_DATA_DIRS for the wayland session files#123
avpcretois wants to merge 2 commits into
kavau:mainfrom
avpcretois:xdg-data-dirs

Conversation

@avpcretois

Copy link
Copy Markdown

Hello,

This pull request is to resolve issue : #121

I created new files to handle the parsing of the XDG_DATA_DIRS.

As this PR is mainly intended to allow a better compatibility with NixOS, I implented a check on existance of a /wayland-sessions directory in each XDG data directory. It avoids to try opening a lot of non-existent directories.

I did not code in C since my teenage years, and never did professionally so I apologize for the potential mistakes I made.

@kavau

kavau commented Aug 31, 2026

Copy link
Copy Markdown
Owner

Hi,

thanks for contributing! This is a useful fix, which mostly does the right thing. Since it's a draft I'll give you some honest feedback now:

The implementation feels overly complex for what it does, and I think it's due to your choice of data structure. A fixed-size array would do the job just as well, and you wouldn't have to worry about pointer plumbing and memory management, which currently half of the code amounts to.

True, we'd lose some flexibility, but I think the reduced complexity outweighs this. In particular there are two blocking bugs that are a direct consequence of this complexity. They both originate with the result = NULL line in the empty path component branch in map_to_next_data_dir (line 33). This line only nulls the local result parameter, not the output parameter.

  1. Infinite loop on an empty path component:
$ XDG_DATA_DIRS='/usr/local/share::/usr/share' build/atrium-list-sessions
# hangs
  1. Heap use-after-free when the empty component follows a non-existent directory:
$ XDG_DATA_DIRS='/nonexistent::/usr/share' build/atrium-list-sessions
free(): invalid pointer
zsh: abort (core dumped)  XDG_DATA_DIRS='/nonexistent::/opt' build/atrium-list-sessions

If you use a fixed-size array, you could avoid all the complexity that led to these errors. There are a couple of minor things as well, but I'll comment on those in a future round of review.

@avpcretois

Copy link
Copy Markdown
Author

Hello,

Thank you for the review and testing !

Initially I used such structure to cope with the shear number of directories in XDG_DATA_DIRS on NixOS.

However by filtering the ones containing a wayland-sessions directory, that number drops to... one. Moreover the there's a fixed number of possible sessions so I agree it's overengineered!

I'm going to redo it with a fixed size array.

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.

2 participants