| Message ID | 75e5392032dbdbdedf8a2b76a7098e4dc1133d82.1640090038.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tweaks to refs/debug.c | expand |
"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Han-Wen Nienhuys <hanwen@google.com> > > This is for consistency with the files backend. Hmmmm. Could you explain what it exactly means? I can see that files_ref_store structure has the .repo member and files_ref_store_create() uses it to remember which repository the ref store is for, but that is an implementation detail that is not exposed outside the files backend, isn't it? To put it differently, what is broken with the current code that leaves the .repo member in refs->base uninitialized? We are presumably helping the caller that wants to know the repository the ref store belongs to via this pointer with this change---what is that caller? > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> > --- > refs/debug.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/refs/debug.c b/refs/debug.c > index cf6ad36fbb0..136cfd7c700 100644 > --- a/refs/debug.c > +++ b/refs/debug.c > @@ -26,6 +26,7 @@ struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_stor > be_copy->name = store->be->name; > trace_printf_key(&trace_refs, "ref_store for %s\n", gitdir); > res->refs = store; > + res->base.repo = store->repo; > base_ref_store_init((struct ref_store *)res, be_copy); > return (struct ref_store *)res; > } Thanks.
On Wed, Dec 22, 2021 at 6:58 AM Junio C Hamano <gitster@pobox.com> wrote: > > "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > From: Han-Wen Nienhuys <hanwen@google.com> > > > > This is for consistency with the files backend. > > Hmmmm. Could you explain what it exactly means? > > I can see that files_ref_store structure has the .repo member and > files_ref_store_create() uses it to remember which repository the > ref store is for, but that is an implementation detail that is not > exposed outside the files backend, isn't it? > > To put it differently, what is broken with the current code that > leaves the .repo member in refs->base uninitialized? We are > presumably helping the caller that wants to know the repository the > ref store belongs to via this pointer with this change---what is > that caller? It's confusing for the base ref_store to have fields that are sometimes set and sometimes not. I sent an alternate take on this as v2; hope you like that better. (sorry, I forgot to update the cover letter.)
diff --git a/refs/debug.c b/refs/debug.c index cf6ad36fbb0..136cfd7c700 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -26,6 +26,7 @@ struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_stor be_copy->name = store->be->name; trace_printf_key(&trace_refs, "ref_store for %s\n", gitdir); res->refs = store; + res->base.repo = store->repo; base_ref_store_init((struct ref_store *)res, be_copy); return (struct ref_store *)res; }