From: Duy Nguyen Date: Fri, 07 Apr 2017 11:20:07 GMT Subject: Re: [PATCH v2 04/20] refs_verify_refname_available(): implement once for all backends Message-ID: In-Reply-To: <22abd274bfdada94b3654a811ee209822640765f.1490966385.git.mhagger@alum.mit.edu> On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty 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 > --- > 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