"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > diff --git a/refs.c b/refs.c > index 156fdcd459..5febfe2281 100644 > --- a/refs.c > +++ b/refs.c > @@ -1774,6 +1774,7 @@ struct ref_store *get_main_ref_store(struct repository *r) > BUG("attempting to get main_ref_store outside of repository"); > > r->refs_private = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS); > + r->refs_private = maybe_debug_wrap_ref_store(r->gitdir, r->refs_private); So the idea is to capture calls to backend, emit tracing events and defer to the real backend? Cute. I like it. > diff --git a/refs/debug.c b/refs/debug.c > new file mode 100644 > index 0000000000..68ae531b45 > --- /dev/null > +++ b/refs/debug.c > @@ -0,0 +1,396 @@ > + > +#include "refs-internal.h" > +#include "trace.h" > + > +static struct trace_key trace_refs = TRACE_KEY_INIT(REFS); > + > +struct debug_ref_store { > + struct ref_store base; > + struct ref_store *refs; > +}; > + > +extern struct ref_storage_be refs_be_debug; > + > +struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_store *store) > +{ > + struct debug_ref_store *res; > + struct ref_storage_be *be_copy; > + > + if (!trace_want(&trace_refs)) { > + return store; > + } > + res = malloc(sizeof(struct debug_ref_store)); > + be_copy = malloc(sizeof(*be_copy)); Not xmalloc() and friends? > + *be_copy = refs_be_debug; > + be_copy->name = store->be->name; I guess we never destroy the ref-store instances so it is OK to assume that it would not cause problems later by sharing pieces of memory with underlying "real" thing like this. > + trace_printf_key(&trace_refs, "ref_store for %s\n", gitdir); > + res->refs = store; > + base_ref_store_init((struct ref_store *)res, be_copy); > + return (struct ref_store *)res; > +} > + ... > +static void print_update(int i, const char *refname, > + const struct object_id *old_oid, > + const struct object_id *new_oid, unsigned int flags, > + unsigned int type, const char *msg) > +{ > + char o[200] = "null"; > + char n[200] = "null"; I thought we had better constant than 200 (later you use 100 in the same patch). I am not sure how I feel about "null"; all places in Git, we tend to use 0{length-of-the-hash} for "no object name on this side", I think. > + if (old_oid) > + oid_to_hex_r(o, old_oid); > + if (new_oid) > + oid_to_hex_r(n, new_oid); > + > + type &= 0xf; /* see refs.h REF_* */ > + flags &= REF_HAVE_NEW | REF_HAVE_OLD | REF_NO_DEREF | > + REF_FORCE_CREATE_REFLOG; > + trace_printf_key(&trace_refs, "%d: %s %s -> %s (F=0x%x, T=0x%x) \"%s\"\n", i, refname, > + o, n, flags, type, msg); > +} > + > +static void print_transaction(struct ref_transaction *transaction) > +{ > + trace_printf_key(&trace_refs, "transaction {\n"); > + for (int i = 0; i < transaction->nr; i++) { We still do not allow variable decl inside the set-up part of a for(;;) statement, if I recall Documentation/CodingGuidelines correctly. > +static int debug_delete_refs(struct ref_store *ref_store, const char *msg, > + struct string_list *refnames, unsigned int flags) > +{ > + struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; > + int res = > + drefs->refs->be->delete_refs(drefs->refs, msg, refnames, flags); > + int i = 0; No need to initialize 'i' here. > + trace_printf_key(&trace_refs, "delete_refs {\n"); > + for (i = 0; i < refnames->nr; i++) > + trace_printf_key(&trace_refs, "%s\n", refnames->items[i].string); > + trace_printf_key(&trace_refs, "}: %d\n", res); > + return res; > +} Thanks.