Re: [PATCH 3/3] Add threaded versions of functions in symlinks.c.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:12
Jared Hance [off-list ref] writes:
check_leading_patch and has_dirs_only_path both always use the default
s/patch/path/;
cache, which could be a caveat for adding parallelism (which is a concern and even a GSoC proposal). This patch implements threaded_check_leading_path and threading threaded_has_dirs_only_path and then implements the nonthreaded functions in terms of their threaded equivalents. No functional should be changed. Signed-off-by: Jared Hance <redacted> ---
Ok, so these FIXME's started at 867f72b (Prepare symlink caching for thread-safety, 2009-07-09) and some of them were taken care of at b9fd284 (Export thread-safe version of 'has_symlink_leading_path()', 2009-07-09), that follows it. The latter left these two FIXMEs behind. I wonder why. I suspect that it is merely because it wasn't necessary to call these two functions from threaded context at all. The patch changes no behaviour and should be safe, I would think.
quoted hunk
cache.h | 2 ++ symlinks.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-)diff --git a/cache.h b/cache.h index baa8852..1113296 100644 --- a/cache.h +++ b/cache.h@@ -950,7 +950,9 @@ struct cache_def { extern int has_symlink_leading_path(const char *name, int len); extern int threaded_has_symlink_leading_path(struct cache_def *, const char *, int); extern int check_leading_path(const char *name, int len); +extern int threaded_check_leading_path(struct cache_def *cache, const char *name, int len); extern int has_dirs_only_path(const char *name, int len, int prefix_len); +extern int threaded_has_dirs_only_path(struct cache_def *cache, const char *name, int len, int prefix_len); extern void schedule_dir_for_removal(const char *name, int len); extern void remove_scheduled_dirs(void);diff --git a/symlinks.c b/symlinks.c index 034943b..2900367 100644 --- a/symlinks.c +++ b/symlinks.c@@ -219,7 +219,20 @@ int has_symlink_leading_path(const char *name, int len) */ int check_leading_path(const char *name, int len) { - struct cache_def *cache = &default_cache; /* FIXME */ + return threaded_check_leading_path(&default_cache, name, len); +} + +/* + * Return zero if path 'name' has a leading symlink component or + * if some leading path component does not exists. + * + * Return -1 if leading path exists and is a directory. + * + * Return path length if leading path exists and is neither a + * directory nor a symlink. + */ +int threaded_check_leading_path(struct cache_def *cache, const char *name, int len) +{ int flags; int match_len = lstat_cache_matchlen(cache, name, len, &flags, FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT);@@ -240,7 +253,18 @@ int check_leading_path(const char *name, int len) */ int has_dirs_only_path(const char *name, int len, int prefix_len) { - struct cache_def *cache = &default_cache; /* FIXME */ + return threaded_has_dirs_only_path(&default_cache, name, len, prefix_len); +} + +/* + * Return non-zero if all path components of 'name' exists as a + * directory. If prefix_len > 0, we will test with the stat() + * function instead of the lstat() function for a prefix length of + * 'prefix_len', thus we then allow for symlinks in the prefix part as + * long as those points to real existing directories. + */ +int threaded_has_dirs_only_path(struct cache_def *cache, const char *name, int len, int prefix_len) +{ return lstat_cache(cache, name, len, FL_DIR|FL_FULLPATH, prefix_len) & FL_DIR;