Re: [PATCH 3/3] Add threaded versions of functions in symlinks.c.
Jared Hance <jaredhance@gmail.com> writes:
> check_leading_patch and has_dirs_only_path both always use the default
Show 8 quoted lines
> 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 <jaredhance@gmail.com>
> ---
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.
Show 64 quoted lines
> 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;
Jared Hance
