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.

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

conf: fix reloading on IN_CREATE #2215

Merged
merged 1 commit into from
Mar 18, 2024
Merged

Conversation

atalii
Copy link
Contributor

@atalii atalii commented Mar 13, 2024

See the linked issue for context.

This is a strange bug, and I honestly don't know why this fix works (or, rather, why the original code was buggy). I wrote some smaller test programs, and operator== generally does seem to work over a flexible array member. I may be missing something obvious, or there may be some UB going on when ->name is inspected in the buffer returned from read(2). In any case, this fix appears to work, though I'd like some external confirmation or refutation of that given how shaky it seems. Thanks!

Commit message below:

If the config file is a symlink, wayfire attempts to watch the parent directory so that, if replaced via a subsequent `ln -sf`, it'll still be notified. When notified, however, wayfire doesn't immediately reload the config file. Rather, as this event may be triggered by any file creation in the directory, we first make sure that the config file really is the one being created anew.

The filename from inotify is stored as a flexible array member in the event struct, though, and this was somehow causing problems when compared with the known std::string basename.

Fixes #2210.

@atalii
Copy link
Contributor Author

atalii commented Mar 17, 2024

The issue was a little less subtle than I thought; the new commit should fix it. Thanks :)

Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks :)

@ammen99
Copy link
Member

ammen99 commented Mar 18, 2024

If the config file is a symlink, wayfire attempts to watch the parent
directory so that, if replaced via a subsequent `ln -sf`, it'll still
be notified. When notified, however, wayfire doesn't immediately
reload the config file. Rather, as this event may be triggered by any
file creation in the directory, we first make sure that the config file
really is the one being created anew.

Unfortunately, ln -sf is special: It'll create a randomly named file in
the directory before renameat(2)ing it to the proper location. So it
does generate an IN_CREATE event, but to know that the file was changed,
we need to additionally monitor for IN_MOVED_TO.

Fixes WayfireWM#2210.
@atalii
Copy link
Contributor Author

atalii commented Mar 18, 2024

@ammen99 ammen99 merged commit 77057b0 into WayfireWM:master Mar 18, 2024
4 checks passed
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