* [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set @ 2021-04-12 19:32 Han-Wen Nienhuys via GitGitGadget 2021-04-12 21:45 ` Junio C Hamano 2021-04-13 12:10 ` [PATCH v2] " Han-Wen Nienhuys via GitGitGadget 0 siblings, 2 replies; 7+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-12 19:32 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> The ref backend API uses errno as a sideband error channel. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- refs: print errno for read_raw_ref if GIT_TRACE_REFS is set The ref backend API uses errno as a sideband error channel. Signed-off-by: Han-Wen Nienhuys hanwen@google.com Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1002%2Fhanwen%2Ferrno-debug-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1002/hanwen/errno-debug-v1 Pull-Request: https://github.com/git/git/pull/1002 refs/debug.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/refs/debug.c b/refs/debug.c index 922e64fa6ad9..576bf98e74ae 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -244,6 +244,7 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname, int res = 0; oidcpy(oid, &null_oid); + errno = 0; res = drefs->refs->be->read_raw_ref(drefs->refs, refname, oid, referent, type); @@ -251,7 +252,9 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname, trace_printf_key(&trace_refs, "read_raw_ref: %s: %s (=> %s) type %x: %d\n", refname, oid_to_hex(oid), referent->buf, *type, res); } else { - trace_printf_key(&trace_refs, "read_raw_ref: %s: %d\n", refname, res); + trace_printf_key(&trace_refs, + "read_raw_ref: %s: %d (errno %d)\n", refname, + res, errno); } return res; } base-commit: 89b43f80a514aee58b662ad606e6352e03eaeee4 -- gitgitgadget ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set 2021-04-12 19:32 [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set Han-Wen Nienhuys via GitGitGadget @ 2021-04-12 21:45 ` Junio C Hamano 2021-04-13 11:58 ` Han-Wen Nienhuys 2021-04-13 12:10 ` [PATCH v2] " Han-Wen Nienhuys via GitGitGadget 1 sibling, 1 reply; 7+ messages in thread From: Junio C Hamano @ 2021-04-12 21:45 UTC (permalink / raw) To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > diff --git a/refs/debug.c b/refs/debug.c > index 922e64fa6ad9..576bf98e74ae 100644 > --- a/refs/debug.c > +++ b/refs/debug.c > @@ -244,6 +244,7 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname, > int res = 0; > > oidcpy(oid, &null_oid); > + errno = 0; > res = drefs->refs->be->read_raw_ref(drefs->refs, refname, oid, referent, > type); > > @@ -251,7 +252,9 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname, > trace_printf_key(&trace_refs, "read_raw_ref: %s: %s (=> %s) type %x: %d\n", > refname, oid_to_hex(oid), referent->buf, *type, res); > } else { > - trace_printf_key(&trace_refs, "read_raw_ref: %s: %d\n", refname, res); > + trace_printf_key(&trace_refs, > + "read_raw_ref: %s: %d (errno %d)\n", refname, > + res, errno); > } OK. Between trace_printf_key() and the return of the call to read_raw_ref() method of the ref backend is only an "if (res == 0)" condition and I do not see anything that might clobber errno. I do wonder if we want strerror(errno) instead of the number, but I can go either way (it's just a trace output). Thanks, will queue. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set 2021-04-12 21:45 ` Junio C Hamano @ 2021-04-13 11:58 ` Han-Wen Nienhuys 2021-04-13 12:02 ` Han-Wen Nienhuys 2021-04-13 20:08 ` Junio C Hamano 0 siblings, 2 replies; 7+ messages in thread From: Han-Wen Nienhuys @ 2021-04-13 11:58 UTC (permalink / raw) To: Junio C Hamano; +Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys On Mon, Apr 12, 2021 at 11:45 PM Junio C Hamano <gitster@pobox.com> wrote: > > @@ -251,7 +252,9 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname, > > trace_printf_key(&trace_refs, "read_raw_ref: %s: %s (=> %s) type %x: %d\n", > > refname, oid_to_hex(oid), referent->buf, *type, res); > > } else { > > - trace_printf_key(&trace_refs, "read_raw_ref: %s: %d\n", refname, res); > > + trace_printf_key(&trace_refs, > > + "read_raw_ref: %s: %d (errno %d)\n", refname, > > + res, errno); > > } > > OK. Between trace_printf_key() and the return of the call to > read_raw_ref() method of the ref backend is only an "if (res == 0)" > condition and I do not see anything that might clobber errno. I don't want to bet on that. Let me send a second round. > I do wonder if we want strerror(errno) instead of the number, but I > can go either way (it's just a trace output). For tracing, it would be most useful if we got the actual symbol (eg. ENOENT). Is there a way to get at that? -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set 2021-04-13 11:58 ` Han-Wen Nienhuys @ 2021-04-13 12:02 ` Han-Wen Nienhuys 2021-04-13 20:08 ` Junio C Hamano 1 sibling, 0 replies; 7+ messages in thread From: Han-Wen Nienhuys @ 2021-04-13 12:02 UTC (permalink / raw) To: Junio C Hamano; +Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys On Tue, Apr 13, 2021 at 1:58 PM Han-Wen Nienhuys <hanwen@google.com> wrote: > > On Mon, Apr 12, 2021 at 11:45 PM Junio C Hamano <gitster@pobox.com> wrote: > > > @@ -251,7 +252,9 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname, > > > trace_printf_key(&trace_refs, "read_raw_ref: %s: %s (=> %s) type %x: %d\n", > > > refname, oid_to_hex(oid), referent->buf, *type, res); > > > } else { > > > - trace_printf_key(&trace_refs, "read_raw_ref: %s: %d\n", refname, res); > > > + trace_printf_key(&trace_refs, > > > + "read_raw_ref: %s: %d (errno %d)\n", refname, > > > + res, errno); > > > } > > > > OK. Between trace_printf_key() and the return of the call to > > read_raw_ref() method of the ref backend is only an "if (res == 0)" > > condition and I do not see anything that might clobber errno. > > I don't want to bet on that. Let me send a second round. oh, ugh. In email, there are two calls, but one is prefixed with '-'. Nevertheless, a bit of paranoia doesn't hurt here. -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set 2021-04-13 11:58 ` Han-Wen Nienhuys 2021-04-13 12:02 ` Han-Wen Nienhuys @ 2021-04-13 20:08 ` Junio C Hamano 2021-04-23 8:34 ` Han-Wen Nienhuys 1 sibling, 1 reply; 7+ messages in thread From: Junio C Hamano @ 2021-04-13 20:08 UTC (permalink / raw) To: Han-Wen Nienhuys; +Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys Han-Wen Nienhuys <hanwen@google.com> writes: >> I do wonder if we want strerror(errno) instead of the number, but I >> can go either way (it's just a trace output). > > For tracing, it would be most useful if we got the actual symbol (eg. > ENOENT). Is there a way to get at that? I do not think there is. And that is the reason why we see everywhere calls to die_errno(), error(..., strerror(errno)), etc. As this is interpolated into trace with untranslated string, I suspect that strerror() is not a good match, so let's live with %d for now. Thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set 2021-04-13 20:08 ` Junio C Hamano @ 2021-04-23 8:34 ` Han-Wen Nienhuys 0 siblings, 0 replies; 7+ messages in thread From: Han-Wen Nienhuys @ 2021-04-23 8:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys On Tue, Apr 13, 2021 at 10:08 PM Junio C Hamano <gitster@pobox.com> wrote: > > Han-Wen Nienhuys <hanwen@google.com> writes: > > >> I do wonder if we want strerror(errno) instead of the number, but I > >> can go either way (it's just a trace output). > > > > For tracing, it would be most useful if we got the actual symbol (eg. > > ENOENT). Is there a way to get at that? > > I do not think there is. > > And that is the reason why we see everywhere calls to die_errno(), > error(..., strerror(errno)), etc. > > As this is interpolated into trace with untranslated string, > I suspect that strerror() is not a good match, so let's live with %d > for now. Great! This topic is marked as Waiting for reviews to conclude. cf. <xmqq4kgbb2ic.fsf@gitster.g> but I don't know what is left to do. Oversight? -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set 2021-04-12 19:32 [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set Han-Wen Nienhuys via GitGitGadget 2021-04-12 21:45 ` Junio C Hamano @ 2021-04-13 12:10 ` Han-Wen Nienhuys via GitGitGadget 1 sibling, 0 replies; 7+ messages in thread From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-13 12:10 UTC (permalink / raw) To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys From: Han-Wen Nienhuys <hanwen@google.com> The ref backend API uses errno as a sideband error channel. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> --- refs: print errno for read_raw_ref if GIT_TRACE_REFS is set The ref backend API uses errno as a sideband error channel. Signed-off-by: Han-Wen Nienhuys hanwen@google.com Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1002%2Fhanwen%2Ferrno-debug-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1002/hanwen/errno-debug-v2 Pull-Request: https://github.com/git/git/pull/1002 Range-diff vs v1: 1: 9b150c5563f9 ! 1: 1e9a8990e7f2 refs: print errno for read_raw_ref if GIT_TRACE_REFS is set @@ Commit message ## refs/debug.c ## @@ refs/debug.c: static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname, + { + struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; int res = 0; ++ int saved_errno = 0; oidcpy(oid, &null_oid); + errno = 0; res = drefs->refs->be->read_raw_ref(drefs->refs, refname, oid, referent, type); ++ saved_errno = errno; -@@ refs/debug.c: static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname, + if (res == 0) { trace_printf_key(&trace_refs, "read_raw_ref: %s: %s (=> %s) type %x: %d\n", refname, oid_to_hex(oid), referent->buf, *type, res); } else { - trace_printf_key(&trace_refs, "read_raw_ref: %s: %d\n", refname, res); + trace_printf_key(&trace_refs, + "read_raw_ref: %s: %d (errno %d)\n", refname, -+ res, errno); ++ res, saved_errno); } return res; } refs/debug.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/refs/debug.c b/refs/debug.c index 922e64fa6ad9..286987b72166 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -242,16 +242,21 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname, { struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; int res = 0; + int saved_errno = 0; oidcpy(oid, &null_oid); + errno = 0; res = drefs->refs->be->read_raw_ref(drefs->refs, refname, oid, referent, type); + saved_errno = errno; if (res == 0) { trace_printf_key(&trace_refs, "read_raw_ref: %s: %s (=> %s) type %x: %d\n", refname, oid_to_hex(oid), referent->buf, *type, res); } else { - trace_printf_key(&trace_refs, "read_raw_ref: %s: %d\n", refname, res); + trace_printf_key(&trace_refs, + "read_raw_ref: %s: %d (errno %d)\n", refname, + res, saved_errno); } return res; } base-commit: 89b43f80a514aee58b662ad606e6352e03eaeee4 -- gitgitgadget ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-23 8:35 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-12 19:32 [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set Han-Wen Nienhuys via GitGitGadget 2021-04-12 21:45 ` Junio C Hamano 2021-04-13 11:58 ` Han-Wen Nienhuys 2021-04-13 12:02 ` Han-Wen Nienhuys 2021-04-13 20:08 ` Junio C Hamano 2021-04-23 8:34 ` Han-Wen Nienhuys 2021-04-13 12:10 ` [PATCH v2] " Han-Wen Nienhuys via GitGitGadget
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).