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

Fixes for the XDG Base Dirs implementation (#330) #419

Merged
merged 3 commits into from
Aug 14, 2022

Conversation

Cavernosa
Copy link
Contributor

@Cavernosa Cavernosa commented Aug 8, 2022

Fixes errors/typos from #330 in order to better comply with the XDG Base Directory Specification:

  • Renamed XDG_CONFIG_DIR to XDG_CONFIG_HOME
  • added $HOME/.config/ as fallback in case XDG_CONFIG_HOME is null, and PWD as last resort.
  • Filename for the PWD fallback now has a dot (.lyxauth instead of lyxauth) just like the other dotfiles in $HOME

I made some tests and it works fine. I'm a complete noob in C so I'd appreciate if someone reviews the code.

Note we should probably not use XDG_CONFIG_HOME at all since lyxauth is far from a config file. Using /run/user/$UID/ or /tmp/ would probably be better, and then maybe use pwd as the very last resort just in case, i don't really know.

renamed `XDG_CONFIG_DIR` -> `XDG_CONFIG_HOME`
added `.config/ly` as fallback as specified in the specification, and pwd as last resort.
Initially assigning `lyxauth` instead of `.lyxauth` so theres no need for a bunch of `else`s
Using `.config` instead of `.config/ly` just like the XDG_CONFIG_HOME fallback, which also makes more sense.
Copy link
Collaborator

@AnErrupTion AnErrupTion left a comment

Choose a reason for hiding this comment

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

Hmm, why storing .lyxauth in the root of .config? I believe having a separate directory for storing ly stuff is better than leaving it in the root of .config.

@Cavernosa
Copy link
Contributor Author

Cavernosa commented Aug 12, 2022

I didn't think it was necessary since it's only 1 file. But yeah having it inside a directory doesn't hurt.
As i said we could remove XDG_CONFIG_HOME and ~/.config altogether but i'm not sure what would replace it... /tmp/ly/ is my biggest bet.

Edit: I quickly put together the /tmp/ly fallback and the changes you requested, pls tell which one you think is better:

	const char* xauth_file = "lyxauth";
	char* xauth_dir = getenv("XDG_RUNTIME_DIR");
	if ((xauth_dir == NULL) || (*xauth_dir == '\0'))
	{
		struct stat sb;
		stat("/tmp", &sb);
		if (S_ISDIR(sb.st_mode))
		{
			char template[] = "/tmp/ly-XXXXXX";
			xauth_dir = mkdtemp(template);
		}
		
		else
		{
			xauth_dir = pwd;
			xauth_file = ".lyxauth";			
		}
	}

The changes you requested (draft. i didn't try to check if ~/.config/ly folder exists as it's easier to just fallback to the pwd, and it doesn't check if a regular file named ly exists but that shouldn't be a big problem):

	const char* xauth_file = "lyxauth";
	char* xauth_dir = getenv("XDG_RUNTIME_DIR");
	if ((xauth_dir == NULL) || (*xauth_dir == '\0'))
	{
		xauth_dir = getenv("XDG_CONFIG_HOME");
		struct stat sb;
		if ((xauth_dir == NULL) || (*xauth_dir == '\0'))
		{
			xauth_dir = strdup(pwd);
			strcat(xauth_dir, "/.config/ly");
			stat(xauth_dir, &sb);
			if (!S_ISDIR(sb.st_mode))
			{
				xauth_dir = pwd;
				xauth_file = ".lyxauth";
			}
		}
		else
		{
			strcat(xauth_dir, "/ly");
			stat(xauth_dir, &sb);
			if (!S_ISDIR(sb.st_mode))
			{
				mkdir(xauth_dir, 0777);
			}
		}
	}

@AnErrupTion
Copy link
Collaborator

I prefer the second one (with .config/ly), I don't really like having the file in /tmp.

Using `.config/ly` (`XDG_CONFIG_HOME/ly`) instead of `.config/` (`XDG_CONFIG_HOME`)
Added many safety checks so it's impossible to break
@Cavernosa
Copy link
Contributor Author

Fixed.

@AnErrupTion AnErrupTion merged commit 59aae77 into fairyglade:master Aug 14, 2022
@Cavernosa Cavernosa deleted the xdg-base-dirs-fix branch August 14, 2022 21:52
@randomllama
Copy link

Why the 777 chmod?

@Cavernosa
Copy link
Contributor Author

I think i forgot to change it lol. Weirdly, in my system the ly folder always appeared as 0755 instead of 0777.

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.

None yet

3 participants