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

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

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

sysdir: don't assume an empty dir is uninitialized#3877
carlosmn merged 1 commit into
masterfrom
ethomson/paths_init

Conversation

@ethomson

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 ethomson force-pushed the ethomson/paths_init branch from ac2db09 to ef02396 Compare August 2, 2016 23:16
Comment thread src/sysdir.c Outdated

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.

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 ethomson force-pushed the ethomson/paths_init branch from ef02396 to 031d34b Compare August 4, 2016 16:27
@ethomson

ethomson commented Aug 4, 2016

Copy link
Copy Markdown
Member Author

@carlosmn carlosmn merged commit d2794b0 into master Aug 4, 2016
@hanwen hanwen mentioned this pull request Aug 8, 2016
4 tasks
@ethomson ethomson deleted the ethomson/paths_init branch January 13, 2017 12:28
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