Re: [PATCH v2 04/20] refs_verify_refname_available(): implement once for all backends

[Duy Nguyen <pclouds@gmail.com>]

On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> It turns out that we can now implement
> `refs_verify_refname_available()` based on the other virtual
> functions, so there is no need for it to be defined at the backend
> level. Instead, define it once in `refs.c` and remove the
> `files_backend` definition.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>  refs.c               | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  refs.h               |  2 +-
>  refs/files-backend.c | 39 +++++-------------------

Much appreciated. This will make future backends simpler to implement as well.

> +       iter = refs_ref_iterator_begin(refs, dirname.buf, 0,
> +                                      DO_FOR_EACH_INCLUDE_BROKEN);
> +       while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
> +               if (skip &&
> +                   string_list_has_string(skip, iter->refname))
> +                       continue;
> +
> +               strbuf_addf(err, "'%s' exists; cannot create '%s'",
> +                           iter->refname, refname);
> +               ok = ref_iterator_abort(iter);

Saving the return code in "ok" seems redundant because you don't use
it. Or did you want to check that ok == ITER_DONE or die() too?

> +               goto cleanup;
> +       }
> +
> +       if (ok != ITER_DONE)
> +               die("BUG: error while iterating over references");
> +
> +       extra_refname = find_descendant_ref(dirname.buf, extras, skip);
> +       if (extra_refname)
> +               strbuf_addf(err, "cannot process '%s' and '%s' at the same time",
> +                           refname, extra_refname);
> +       else
> +               ret = 0;
> +
> +cleanup:
> +       strbuf_release(&referent);
> +       strbuf_release(&dirname);
> +       return ret;
>  }
-- 
Duy