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

Use XDG_STATE_HOME for jackdbus logs #967

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

keithbowes
Copy link

Fixes #402

In that bug, it's mentioned that Debian has an $XDG_STATE_HOME extension for that purpose. Well, it's been added to version 0.8 of the specification (8 May 2021) with the description:

The $XDG_STATE_HOME contains state data that should persist between (application) restarts, but that is not important or portable enough to the user that it should be stored in $XDG_DATA_HOME. It may contain:

  • actions history (logs, history, recently used files, …)
  • current state of the application that can be reused on a restart (view, layout, open files, undo history, …)

It's been a part of the official specification for almost three years and other applications use it, so it should be safe to use it and not add unnecessary files and directories to $HOME.

dbus/jackdbus.c Outdated
@@ -694,7 +694,7 @@ pathname_cat(const char *pathname_a, const char *pathname_b)
bool
paths_init()
{
const char *home_dir, *xdg_config_home, *xdg_log_home;
const char *home_dir, *xdg_config_home, *xdg_state_home;
Copy link
Contributor

Choose a reason for hiding this comment

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

Tabs are the wrong indentation style here. The whole file contains just a handful of tab-indented lines (which have been like that since 2008), all the rest is indented with 4 spaces. Now would be a good time to not replicate the exact original indentation for the changed lines, but instead to fix all of it. Please use 4 spaces for your changes,, and also fix the remaining old tab-indented lines in the function, there's like 5 of them.

dbus/jackdbus.c Outdated
xdg_state_home = getenv("XDG_STATE_HOME");
if (xdg_state_home == NULL)
{
if (!(xdg_state_home = pathname_cat(home_dir, DEFAULT_XDG_STATE))) goto fail;
Copy link
Contributor

Choose a reason for hiding this comment

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

This leaks memory and is problematic in general. A pointer from getenv() shouldn't be free() d, but pathname_cat() returns an allocated buffer pointer which definitely should. If you use a single pointer variable like this, you lose information about whether you need to call free() on it before returning.

That being said, none of the temporary allocations from pathname_cat() are ever freed. The whole thing seems to have been leaky since 2008.

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.

Make the log output directory XDG compliant or configurable
2 participants