{"thread":{"id":"55483","subject":"[PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set","startedAt":"2021-04-12T19:32:39Z","lastAt":"2021-04-23T08:35:14Z","messageCount":7,"participants":["Han-Wen Nienhuys via GitGitGadget","Junio C Hamano","Han-Wen Nienhuys"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"421776","messageId":"pull.1002.git.git.1618255954484.gitgitgadget@gmail.com","threadId":"55483","inReplyTo":null,"subject":"[PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-04-12T19:32:34Z","receivedAt":"2021-04-12T19:32:39Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe ref backend API uses errno as a sideband error channel.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n refs: print errno for read_raw_ref if GIT_TRACE_REFS is set\n \n The ref backend API uses errno as a sideband error channel.\n \n Signed-off-by: Han-Wen Nienhuys hanwen@google.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1002%2Fhanwen%2Ferrno-debug-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1002/hanwen/errno-debug-v1\nPull-Request: https://github.com/git/git/pull/1002\n\n refs/debug.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 922e64fa6ad9..576bf98e74ae 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -244,6 +244,7 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \tint res = 0;\n \n \toidcpy(oid, &null_oid);\n+\terrno = 0;\n \tres = drefs->refs->be->read_raw_ref(drefs->refs, refname, oid, referent,\n \t\t\t\t\t type);\n \n@@ -251,7 +252,9 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,\n \t\ttrace_printf_key(&trace_refs, \"read_raw_ref: %s: %s (=> %s) type %x: %d\\n\",\n \t\t\trefname, oid_to_hex(oid), referent->buf, *type, res);\n \t} else {\n-\t\ttrace_printf_key(&trace_refs, \"read_raw_ref: %s: %d\\n\", refname, res);\n+\t\ttrace_printf_key(&trace_refs,\n+\t\t\t\t \"read_raw_ref: %s: %d (errno %d)\\n\", refname,\n+\t\t\t\t res, errno);\n \t}\n \treturn res;\n }\n\nbase-commit: 89b43f80a514aee58b662ad606e6352e03eaeee4\n-- \ngitgitgadget\n"},{"id":"421817","messageId":"xmqq4kgbb2ic.fsf@gitster.g","threadId":"55483","inReplyTo":"pull.1002.git.git.1618255954484.gitgitgadget@gmail.com","subject":"Re: [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-12T21:45:47Z","receivedAt":"2021-04-12T21:45:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" writes:\n\n> diff --git a/refs/debug.c b/refs/debug.c\n> index 922e64fa6ad9..576bf98e74ae 100644\n> --- a/refs/debug.c\n> +++ b/refs/debug.c\n> @@ -244,6 +244,7 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,\n> \tint res = 0;\n> \n> \toidcpy(oid, &null_oid);\n> +\terrno = 0;\n> \tres = drefs->refs->be->read_raw_ref(drefs->refs, refname, oid, referent,\n> \t\t\t\t\t type);\n> \n> @@ -251,7 +252,9 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,\n> \t\ttrace_printf_key(&trace_refs, \"read_raw_ref: %s: %s (=> %s) type %x: %d\\n\",\n> \t\t\trefname, oid_to_hex(oid), referent->buf, *type, res);\n> \t} else {\n> -\t\ttrace_printf_key(&trace_refs, \"read_raw_ref: %s: %d\\n\", refname, res);\n> +\t\ttrace_printf_key(&trace_refs,\n> +\t\t\t\t \"read_raw_ref: %s: %d (errno %d)\\n\", refname,\n> +\t\t\t\t res, errno);\n> \t}\n\nOK. Between trace_printf_key() and the return of the call to\nread_raw_ref() method of the ref backend is only an \"if (res == 0)\"\ncondition and I do not see anything that might clobber errno.\n\nI do wonder if we want strerror(errno) instead of the number, but I\ncan go either way (it's just a trace output).\n\nThanks, will queue.\n"},{"id":"421877","messageId":"CAFQ2z_MnZOfo4Bt6SsRJnL+F70r9wr_jYSf2KGvcvyX-8PfVRQ@mail.gmail.com","threadId":"55483","inReplyTo":"xmqq4kgbb2ic.fsf@gitster.g","subject":"Re: [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2021-04-13T11:58:42Z","receivedAt":"2021-04-13T11:58:55Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Mon, Apr 12, 2021 at 11:45 PM Junio C Hamano wrote:\n> > @@ -251,7 +252,9 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,\n> > trace_printf_key(&trace_refs, \"read_raw_ref: %s: %s (=> %s) type %x: %d\\n\",\n> > refname, oid_to_hex(oid), referent->buf, *type, res);\n> > } else {\n> > - trace_printf_key(&trace_refs, \"read_raw_ref: %s: %d\\n\", refname, res);\n> > + trace_printf_key(&trace_refs,\n> > + \"read_raw_ref: %s: %d (errno %d)\\n\", refname,\n> > + res, errno);\n> > }\n>\n> OK. Between trace_printf_key() and the return of the call to\n> read_raw_ref() method of the ref backend is only an \"if (res == 0)\"\n> condition and I do not see anything that might clobber errno.\n\nI don't want to bet on that. Let me send a second round.\n\n> I do wonder if we want strerror(errno) instead of the number, but I\n> can go either way (it's just a trace output).\n\nFor tracing, it would be most useful if we got the actual symbol (eg.\nENOENT). Is there a way to get at that?\n\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"421878","messageId":"CAFQ2z_OMUdi4D3Jg3rsPr_Q-VeRptdiEOiYOL2kmyuBuvZLCjg@mail.gmail.com","threadId":"55483","inReplyTo":"CAFQ2z_MnZOfo4Bt6SsRJnL+F70r9wr_jYSf2KGvcvyX-8PfVRQ@mail.gmail.com","subject":"Re: [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2021-04-13T12:02:25Z","receivedAt":"2021-04-13T12:02:40Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Tue, Apr 13, 2021 at 1:58 PM Han-Wen Nienhuys wrote:\n>\n> On Mon, Apr 12, 2021 at 11:45 PM Junio C Hamano wrote:\n> > > @@ -251,7 +252,9 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,\n> > > trace_printf_key(&trace_refs, \"read_raw_ref: %s: %s (=> %s) type %x: %d\\n\",\n> > > refname, oid_to_hex(oid), referent->buf, *type, res);\n> > > } else {\n> > > - trace_printf_key(&trace_refs, \"read_raw_ref: %s: %d\\n\", refname, res);\n> > > + trace_printf_key(&trace_refs,\n> > > + \"read_raw_ref: %s: %d (errno %d)\\n\", refname,\n> > > + res, errno);\n> > > }\n> >\n> > OK. Between trace_printf_key() and the return of the call to\n> > read_raw_ref() method of the ref backend is only an \"if (res == 0)\"\n> > condition and I do not see anything that might clobber errno.\n>\n> I don't want to bet on that. Let me send a second round.\n\noh, ugh. In email, there are two calls, but one is prefixed with '-'.\n\nNevertheless, a bit of paranoia doesn't hurt here.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"421881","messageId":"pull.1002.v2.git.git.1618315838582.gitgitgadget@gmail.com","threadId":"55483","inReplyTo":"pull.1002.git.git.1618255954484.gitgitgadget@gmail.com","subject":"[PATCH v2] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-04-13T12:10:38Z","receivedAt":"2021-04-13T12:10:42Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe ref backend API uses errno as a sideband error channel.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n refs: print errno for read_raw_ref if GIT_TRACE_REFS is set\n \n The ref backend API uses errno as a sideband error channel.\n \n Signed-off-by: Han-Wen Nienhuys hanwen@google.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1002%2Fhanwen%2Ferrno-debug-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1002/hanwen/errno-debug-v2\nPull-Request: https://github.com/git/git/pull/1002\n\nRange-diff vs v1:\n\n 1: 9b150c5563f9 ! 1: 1e9a8990e7f2 refs: print errno for read_raw_ref if GIT_TRACE_REFS is set\n @@ Commit message\n \n ## refs/debug.c ##\n @@ refs/debug.c: static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,\n + {\n + \tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n \tint res = 0;\n ++\tint saved_errno = 0;\n \n \toidcpy(oid, &null_oid);\n +\terrno = 0;\n \tres = drefs->refs->be->read_raw_ref(drefs->refs, refname, oid, referent,\n \t\t\t\t\t type);\n ++\tsaved_errno = errno;\n \n -@@ refs/debug.c: static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,\n + \tif (res == 0) {\n \t\ttrace_printf_key(&trace_refs, \"read_raw_ref: %s: %s (=> %s) type %x: %d\\n\",\n \t\t\trefname, oid_to_hex(oid), referent->buf, *type, res);\n \t} else {\n -\t\ttrace_printf_key(&trace_refs, \"read_raw_ref: %s: %d\\n\", refname, res);\n +\t\ttrace_printf_key(&trace_refs,\n +\t\t\t\t \"read_raw_ref: %s: %d (errno %d)\\n\", refname,\n -+\t\t\t\t res, errno);\n ++\t\t\t\t res, saved_errno);\n \t}\n \treturn res;\n }\n\n\n refs/debug.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 922e64fa6ad9..286987b72166 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -242,16 +242,21 @@ static int debug_read_raw_ref(struct ref_store *ref_store, const char *refname,\n {\n \tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n \tint res = 0;\n+\tint saved_errno = 0;\n \n \toidcpy(oid, &null_oid);\n+\terrno = 0;\n \tres = drefs->refs->be->read_raw_ref(drefs->refs, refname, oid, referent,\n \t\t\t\t\t type);\n+\tsaved_errno = errno;\n \n \tif (res == 0) {\n \t\ttrace_printf_key(&trace_refs, \"read_raw_ref: %s: %s (=> %s) type %x: %d\\n\",\n \t\t\trefname, oid_to_hex(oid), referent->buf, *type, res);\n \t} else {\n-\t\ttrace_printf_key(&trace_refs, \"read_raw_ref: %s: %d\\n\", refname, res);\n+\t\ttrace_printf_key(&trace_refs,\n+\t\t\t\t \"read_raw_ref: %s: %d (errno %d)\\n\", refname,\n+\t\t\t\t res, saved_errno);\n \t}\n \treturn res;\n }\n\nbase-commit: 89b43f80a514aee58b662ad606e6352e03eaeee4\n-- \ngitgitgadget\n"},{"id":"421924","messageId":"xmqqy2dmkkvo.fsf@gitster.g","threadId":"55483","inReplyTo":"CAFQ2z_MnZOfo4Bt6SsRJnL+F70r9wr_jYSf2KGvcvyX-8PfVRQ@mail.gmail.com","subject":"Re: [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-13T20:08:43Z","receivedAt":"2021-04-13T20:08:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n>> I do wonder if we want strerror(errno) instead of the number, but I\n>> can go either way (it's just a trace output).\n>\n> For tracing, it would be most useful if we got the actual symbol (eg.\n> ENOENT). Is there a way to get at that?\n\nI do not think there is.\n\nAnd that is the reason why we see everywhere calls to die_errno(),\nerror(..., strerror(errno)), etc.\n\nAs this is interpolated into trace with untranslated string,\nI suspect that strerror() is not a good match, so let's live with %d\nfor now.\n\nThanks.\n\n\n"},{"id":"422733","messageId":"CAFQ2z_PhkOEnKNFZ29R5x9rttO+3POT8Tdx9stcQ__oNG=wQsA@mail.gmail.com","threadId":"55483","inReplyTo":"xmqqy2dmkkvo.fsf@gitster.g","subject":"Re: [PATCH] refs: print errno for read_raw_ref if GIT_TRACE_REFS is set","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2021-04-23T08:34:59Z","receivedAt":"2021-04-23T08:35:14Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Tue, Apr 13, 2021 at 10:08 PM Junio C Hamano wrote:\n>\n> Han-Wen Nienhuys writes:\n>\n> >> I do wonder if we want strerror(errno) instead of the number, but I\n> >> can go either way (it's just a trace output).\n> >\n> > For tracing, it would be most useful if we got the actual symbol (eg.\n> > ENOENT). Is there a way to get at that?\n>\n> I do not think there is.\n>\n> And that is the reason why we see everywhere calls to die_errno(),\n> error(..., strerror(errno)), etc.\n>\n> As this is interpolated into trace with untranslated string,\n> I suspect that strerror() is not a good match, so let's live with %d\n> for now.\n\nGreat!\n\nThis topic is marked as\n\nWaiting for reviews to conclude.\ncf. \n\nbut I don't know what is left to do. Oversight?\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\nRegistergericht und -nummer: Hamburg, HRB 86891\nSitz der Gesellschaft: Hamburg\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"}]}