FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

sysdir: don't assume an empty dir is uninitialized by ethomson · Pull Request #3877 · libgit2/libgit2 · GitHub

Repository navigation

sysdir: don't assume an empty dir is uninitialized - #3877

Merged
carlosmn merged 1 commit into
masterfrom
ethomson/paths_init
Aug 4, 2016
Merged

carlosmn merged 1 commit into
masterfrom
ethomson/paths_init

Conversation

Copy link
Copy Markdown
Member

Don't assume that an empty system directory buffer is uninitialized.
In fact, there simply may be no appropriate path for that value.
(For example, the Windows-specific programdata directory has no value
on non-Windows machines.)

This prevents us from continually trying to re-lookup these values,
which could get racy if two different threads are each calling
git_sysdir_get and trying to lookup / clear the value simultaneously.

Fixes #3871

ethomson force-pushed the ethomson/paths_init branch from ac2db09 to ef02396 Compare August 2, 2016 23:16
Comment thread src/sysdir.c Outdated
git_buf buf;
};

static struct git_sysdir__dir git_sysdir__dirs[GIT_SYSDIR__MAX];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hum. If you're already creating a new sysdir struct, what do you think about including the guess callbacks into the struct and then drop the git_sysdir__dir_guess array? So basically

static struct git_sysdir__dir dirs[] = {
    { git_sysdir_guess_system_dirs, GIT_BUF_INIT },
    { git_sysdir_guess_global_dirs, GIT_BUF_INIT },
    ...
};

This would make the correlation of these two structures clearer and reduce a bit of its magic.

Don't try to determine when sysdirs are uninitialized.  Instead, simply
initialize them all at `git_libgit2_init` time and never try to
reinitialize, except when consumers explicitly call `git_sysdir_set`.

Looking at the buffer length is especially problematic, since there may
no appropriate path for that value.  (For example, the Windows-specific
programdata directory has no value on non-Windows machines.)

Previously we would continually trying to re-lookup these values,
which could get racy if two different threads are each calling
`git_sysdir_get` and trying to lookup / clear the value simultaneously.
ethomson force-pushed the ethomson/paths_init branch from ef02396 to 031d34b Compare August 4, 2016 16:27

ethomson commented Aug 4, 2016

Copy link
Copy Markdown
Member Author

Thanks to both @pks-t and @hanwen, I think that this is a big improvement. I've incorporated both of your fine ideas.

carlosmn merged commit d2794b0 into master Aug 4, 2016
hanwen mentioned this pull request Aug 8, 2016
4 tasks done
ethomson deleted the ethomson/paths_init branch January 13, 2017 12:28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

4 participants


Back | FazBrowse Home | New Git URL