git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[RFC PATCH 00/21] C99: show meaningful <file>:<line> in trace2 via macros

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 15, 2021, 22:18 UTC
Message-ID
<RFC-cover-00.21-00000000000-20211115T220831Z-avarab@gmail.com>
Since everyone's getting in on the C99 fun.

Well, $subject and a bit more. This RFC series has bits and pieces from thing I've submitted before. I'd proposed to make variadic macros a hard dependency before in [1] because I wanted to get to the goal in $subject, perhaps the whole thing will be more convincing.

This also includes the die_message() in a recent series of mine[2] that I abandoned.

At the end of this series we expose a config variable to have usage/die/warning emit line numbers. I.e. going from:

    $ git -c core.usageAddSource=false -c core.x=y config --get --bool core.x
    fatal: bad boolean config value 'y' for 'core.x'
To:
    $ git -c core.usageAddSource=false -c core.x=y config --get --bool core.x
    fatal: config.c:1241: bad boolean config value 'y' for 'core.x'

I find that to make tracing down errors in the test suite, and 21/21 has a GIT_TEST_* mode to turn it on there (which fails a lot now, but I'm hoping I'll eventually get passing).

But most importantly we've now got meaningful file/line numbers in
trace2 error events. I.e. from all of them being some line in usage.c:
    
    $ GIT_TRACE2_EVENT=/dev/stdout ~/g/git/git -c core.usageAddSource=false -c core.x=y config --get --bool core.x 2>&1 2>/dev/null|grep error | jq -r .
    {
      "event": "error",
      "sid": "20211115T221343.534151Z-Hc2f5b994-P003f3980",
      "thread": "main",
      "time": "2021-11-15T22:13:43.537981Z",
      "file": "usage.c",
      "line": 65,
      "msg": "bad boolean config value 'y' for 'core.x'",
      "fmt": "bad boolean config value '%s' for '%s'"
    }
To:
    
    $ GIT_TRACE2_EVENT=/dev/stdout ~/g/git/git -c core.usageAddSource=false -c core.x=y config --get --bool core.x 2>&1 2>/dev/null|grep error | jq -r .
    {
      "event": "error",
      "sid": "20211115T221357.083824Z-Hc2f5b994-P003f4a82",
      "thread": "main",
      "time": "2021-11-15T22:13:57.087596Z",
      "file": "config.c",
      "line": 1241,
      "msg": "bad boolean config value 'y' for 'core.x'",
      "fmt": "bad boolean config value '%s' for '%s'"
    }

I've got some speculation in 19/21 that this may make the "fmt" part redundant, i.e. did we only add that because we couldn't group these by file/line, but as noted there there's still some use-cases for "fmt" even with this series. In any case, this series doesn't touch that "fmt" key at all.

This is "RFC" mainly because there's a CI failure in 0061.2 with this, I still can't figure out what that's about (or if it's some fluke unrelated to this topic), but that has to be investigated.

But I wanted to see if people found the general idea interesting too. I picked the CC list mainly from paging through "--grep=trace2", and people who'd modified the tricker bits of usage.c code being modified here.

1. https://lore.kernel.org/git/cover-0.2-00000000000-20210412T105422Z-avarab@gmail.com/
2. https://lore.kernel.org/git/cover-v3-0.6-00000000000-20211022T175227Z-avarab@gmail.com/
3. https://github.com/avar/git/runs/4216916706?check_suite_focus=true
Ævar Arnfjörð Bjarmason (21):
  git-compat-util.h: clarify GCC v.s. C99-specific in comment
  C99 support: hard-depend on C99 variadic macros
  usage.c: add a die_message() routine
  usage.c API users: use die_message() where appropriate
  usage.c + gc: add and use a die_message_errno()
  config API: don't use vreportf(), make it static in usage.c
  common-main.c: call exit(), don't return
  usage.c: add a non-fatal bug() function to go with BUG()
  parse-options.[ch] API: use bug() to improve error output
  receive-pack: use bug() and BUG_if_bug()
  cache-tree.c: use bug() and BUG_if_bug()
  pack-objects: use BUG(...) not die("BUG: ...")
  strbuf.h: use BUG(...) not die("BUG: ...")
  usage API: create a new usage.h, move API docs there
  usage.[ch] API users: use report_fn, not hardcoded prototype
  usage.[ch] API: rename "warn" vars functions to "warning"
  usage.c: move usage routines around
  usage.c: move rename variables in usage routines around
  usage API: use C99 macros for {usage,usagef,die,error,warning,die}*()
  usage API: make the "{usage,fatal,error,warning,BUG}: " translatable
  usage API: add "core.usageAddSource" config to add <file>:<line>
 Documentation/CodingGuidelines                |   3 +
 Documentation/config/core.txt                 |   7 +
 .../technical/api-error-handling.txt          |  81 ------
 Documentation/technical/api-trace2.txt        |   4 +-
 apply.c                                       |   8 +-
 apply.h                                       |   6 +-
 banned.h                                      |   5 -
 builtin/fast-import.c                         |  22 +-
 builtin/gc.c                                  |  21 +-
 builtin/notes.c                               |  15 +-
 builtin/pack-objects.c                        |   2 +-
 builtin/receive-pack.c                        |  16 +-
 cache-tree.c                                  |   7 +-
 common-main.c                                 |   9 +-
 config.c                                      |  22 +-
 config.h                                      |  10 +-
 daemon.c                                      |   3 +-
 git-compat-util.h                             |  59 +---
 http-backend.c                                |   6 +-
 imap-send.c                                   |   3 +-
 parse-options.c                               |  56 ++--
 repo-settings.c                               |  11 +
 repository.h                                  |   2 +
 run-command.c                                 |  32 +--
 strbuf.h                                      |   2 +-
 t/helper/test-trace2.c                        |  27 +-
 t/t0210-trace2-normal.sh                      |  52 ++++
 trace.c                                       |  80 +-----
 trace.h                                       | 133 ++++-----
 trace2.c                                      |  45 +--
 trace2.h                                      |  28 --
 usage.c                                       | 270 +++++++++++-------
 usage.h                                       | 180 ++++++++++++
 33 files changed, 636 insertions(+), 591 deletions(-)
 delete mode 100644 Documentation/technical/api-error-handling.txt
 create mode 100644 usage.h
-- 
2.34.0.rc2.809.g11e21d44b24
Next: Ævar Arnfjörð Bjarmason
Message 1 of 34 in “C99: show meaningful <file>:<line> in trace2 via macros”
  1. 00/21 C99: show meaningful <file>:<line> in trace2 via macrosÆvar Arnfjörð Bjarmason, Nov 15, 2021
  2. 01/21 git-compat-util.h: clarify GCC v.s. C99-specific in commentÆvar Arnfjörð Bjarmason, Nov 15, 2021
  3. 03/21 usage.c: add a die_message() routineÆvar Arnfjörð Bjarmason, Nov 15, 2021
  4. 02/21 C99 support: hard-depend on C99 variadic macrosÆvar Arnfjörð Bjarmason, Nov 15, 2021
  5. 04/21 usage.c API users: use die_message() where appropriateÆvar Arnfjörð Bjarmason, Nov 15, 2021
  6. 05/21 usage.c + gc: add and use a die_message_errno()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  7. 07/21 common-main.c: call exit(), don't returnÆvar Arnfjörð Bjarmason, Nov 15, 2021
  8. 06/21 config API: don't use vreportf(), make it static in usage.cÆvar Arnfjörð Bjarmason, Nov 15, 2021
  9. 08/21 usage.c: add a non-fatal bug() function to go with BUG()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  10. 10/21 receive-pack: use bug() and BUG_if_bug()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  11. 11/21 cache-tree.c: use bug() and BUG_if_bug()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  12. 12/21 pack-objects: use BUG(...) not die("BUG: ...")Ævar Arnfjörð Bjarmason, Nov 15, 2021
  13. 17/21 usage.c: move usage routines aroundÆvar Arnfjörð Bjarmason, Nov 15, 2021
  14. 21/21 usage API: add "core.usageAddSource" config to add <file>:<line>Ævar Arnfjörð Bjarmason, Nov 15, 2021
  15. 20/21 usage API: make the "{usage,fatal,error,warning,BUG}: " translatableÆvar Arnfjörð Bjarmason, Nov 15, 2021
  16. 19/21 usage API: use C99 macros for {usage,usagef,die,error,warning,die}*()Ævar Arnfjörð Bjarmason, Nov 15, 2021
  17. Jeff HostetlerDec 27, 2021
  18. Ævar Arnfjörð BjarmasonDec 27, 2021
  19. Jeff HostetlerDec 28, 2021
  20. Elijah NewrenDec 28, 2021
  21. Ævar Arnfjörð BjarmasonDec 28, 2021
  22. Elijah NewrenDec 29, 2021
  23. Ævar Arnfjörð BjarmasonDec 28, 2021
  24. Jeff HostetlerDec 29, 2021
  25. 18/21 usage.c: move rename variables in usage routines aroundÆvar Arnfjörð Bjarmason, Nov 15, 2021
  26. 16/21 usage.[ch] API: rename "warn" vars functions to "warning"Ævar Arnfjörð Bjarmason, Nov 15, 2021
  27. 14/21 usage API: create a new usage.h, move API docs thereÆvar Arnfjörð Bjarmason, Nov 15, 2021
  28. 15/21 usage.[ch] API users: use report_fn, not hardcoded prototypeÆvar Arnfjörð Bjarmason, Nov 15, 2021
  29. 13/21 strbuf.h: use BUG(...) not die("BUG: ...")Ævar Arnfjörð Bjarmason, Nov 15, 2021
  30. 09/21 parse-options.[ch] API: use bug() to improve error outputÆvar Arnfjörð Bjarmason, Nov 15, 2021
  31. Taylor BlauNov 16, 2021
  32. Ævar Arnfjörð BjarmasonNov 16, 2021
  33. Taylor BlauNov 16, 2021
  34. Ævar Arnfjörð BjarmasonNov 16, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.