Re: [PATCH v3 01/44] refs.c: create a public version of verify_refname_available

[Michael Haggerty <mhagger@alum.mit.edu>]

On 10/12/2015 11:51 PM, David Turner wrote:
> From: Ronnie Sahlberg <sahlberg@google.com>
> 
> Create a public version of verify_refname_available that backends can
> provide.
> 
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
>  refs.c | 65 ++++++++++++++++++++++++++++++-----------------------------------
>  refs.h | 17 +++++++++++++++++
>  2 files changed, 47 insertions(+), 35 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index 132eff5..fe71ea0 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -279,7 +279,7 @@ struct ref_dir {
>   * presence of an empty subdirectory does not block the creation of a
>   * similarly-named reference.  (The fact that reference names with the
>   * same leading components can conflict *with each other* is a
> - * separate issue that is regulated by verify_refname_available().)
> + * separate issue that is regulated by verify_refname_available_dir().)
>   *
>   * Please note that the name field contains the fully-qualified
>   * reference (or subdirectory) name.  Space could be saved by only
> @@ -897,25 +897,13 @@ static int nonmatching_ref_fn(struct ref_entry *entry, void *vdata)
>  /*
>   * Return 0 if a reference named refname could be created without
>   * conflicting with the name of an existing reference in dir.
> - * Otherwise, return a negative value and write an explanation to err.
> - * If extras is non-NULL, it is a list of additional refnames with
> - * which refname is not allowed to conflict. If skip is non-NULL,
> - * ignore potential conflicts with refs in skip (e.g., because they
> - * are scheduled for deletion in the same operation). Behavior is
> - * undefined if the same name is listed in both extras and skip.
> - *
> - * Two reference names conflict if one of them exactly matches the
> - * leading components of the other; e.g., "refs/foo/bar" conflicts
> - * with both "refs/foo" and with "refs/foo/bar/baz" but not with
> - * "refs/foo/bar" or "refs/foo/barbados".
> - *
> - * extras and skip must be sorted.
> + * See verify_refname_available for details.
>   */
> -static int verify_refname_available(const char *refname,
> -				    const struct string_list *extras,
> -				    const struct string_list *skip,
> -				    struct ref_dir *dir,
> -				    struct strbuf *err)
> +static int verify_refname_available_dir(const char *refname,
> +					const struct string_list *extras,
> +					const struct string_list *skip,
> +					struct ref_dir *dir,
> +					struct strbuf *err)
>  {
>  	const char *slash;
>  	int pos;
> @@ -2464,9 +2452,12 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
>  		 */
>  		strbuf_git_path(&orig_ref_file, "%s", orig_refname);
>  		if (remove_empty_directories(&orig_ref_file)) {
> +			struct ref_dir *loose_refs;
> +			loose_refs = get_loose_refs(&ref_cache);

It's OK to initialize `loose_refs` on the line where it is declared.

>  			last_errno = errno;
> -			if (!verify_refname_available(orig_refname, extras, skip,
> -						      get_loose_refs(&ref_cache), err))
> +			if (!verify_refname_available_dir(orig_refname, extras,
> +							  skip, loose_refs,
> +							  err))
>  				strbuf_addf(err, "there are still refs under '%s'",
>  					    orig_refname);
>  			goto error_return;
> [...]
> diff --git a/refs.h b/refs.h
> index 6d30c98..79ea220 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -218,6 +218,23 @@ extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct st
>  int pack_refs(unsigned int flags);
>  
>  /*
> + * Return true iff a reference named refname could be created without
> + * conflicting with the name of an existing reference.  If
> + * skip is non-NULL, ignore potential conflicts with refs in skip
> + * (e.g., because they are scheduled for deletion in the same
> + * operation).
> + *
> + * Two reference names conflict if one of them exactly matches the
> + * leading components of the other; e.g., "foo/bar" conflicts with
> + * both "foo" and with "foo/bar/baz" but not with "foo/bar" or
> + * "foo/barbados".
> + *
> + * skip must be sorted.
> + */
> +int verify_refname_available(const char *newname, struct string_list *extra,
> +			     struct string_list *skip, struct strbuf *err);

This function has an `extra` parameter but the description of this
parameter disappeared as the docstring was moved from refs.c to here.

> +
> +/*
>   * Flags controlling ref_transaction_update(), ref_transaction_create(), etc.
>   * REF_NODEREF: act on the ref directly, instead of dereferencing
>   *              symbolic references.
> 

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu