git/usage.c, branch v2.45.3

usage: report vsnprintf(3) failure

2024-04-05T22:16:27Z

vreportf(), which is used e.g. by die() and warning() by default, calls vsnprintf(3) to format the message to report. If that call fails, it only prints the prefix, e.g. "fatal: " or "warning: ". This at least informs users that they were supposed to get a message and reveals its severity, but leaves them wondering what it may have been about. Here's an example where vreportf() tries to print a message with a 2GB string, which is too much for vsnprintf(3): $ perl -le 'print "create refs/heads/", "a"x2**31' | git update-ref --stdin fatal: At least report the formatting error along with the offending message (unformatted) to indicate why that message is empty. Use fprintf(3) instead of error() to get the message out directly and avoid recursing back into vreportf(). With this patch we get: $ perl -le 'print "create refs/heads/", "a"x2**31' | git update-ref --stdin error: unable to format message: invalid ref format: %s fatal: ... which allows users to at least get an idea of what went wrong. Suggested-by: Jeff King Helped-by: Jeff King Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

treewide: remove unnecessary includes for wrapper.h

2023-07-05T18:41:59Z

Signed-off-by: Calvin Wan Signed-off-by: Junio C Hamano

treewide: remove unnecessary includes of cache.h

2023-03-21T17:56:53Z

The last several commits were geared at replacing the include of cache.h in strbuf.c with an include of git-compat-util.h. Unfortunately, I had to drop a patch moving some functions from cache.h to object-name.h, due to excessive conflicts with other in-flight topics. However, even without that patch, the series of patches so far allows us to modify a number of C files to replace an include of cache.h with git-compat-util.h. Do that to reduce our dependencies. (If we could have kept our object-name.h patch in this series, it would have also let us reduce the includes in checkout.c and fmt-merge-msg.c in addition to strbuf.c). Just to ensure that nothing else was bringing in cache.h, all of the affected files have been checked to ensure that gcc -E -I. $SOURCE_FILE | grep '"cache.h"' found no hits and that make DEVELOPER=1 ${OBJECT_FILE_FOR_SOURCE_FILE} successfully compiles without warnings. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

wrapper.h: move declarations for wrapper.c functions from cache.h

2023-03-21T17:56:53Z

Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

treewide: be explicit about dependence on gettext.h

2023-03-21T17:56:51Z

Dozens of files made use of gettext functions, without explicitly including gettext.h. This made it more difficult to find which files could remove a dependence on cache.h. Make C files explicitly include gettext.h if they are using it. However, while compat/fsmonitor/fsm-ipc-darwin.c should also gain an include of gettext.h, it was left out to avoid conflicting with an in-flight topic. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

i18n: mark message helpers prefix for translation

2022-06-21T17:06:54Z

Some messages prefixes like 'usage:'/'fatal:'/'warning:'/'error:' were not translated. Signed-off-by: Dimiytriy Ryazantcev Signed-off-by: Junio C Hamano

bug_fl(): correctly initialize trace2 va_list

2022-06-16T20:28:22Z

The code added 0cc05b044f (usage.c: add a non-fatal bug() function to go with BUG(), 2022-06-02) sets up two va_list variables: one to output to stderr, and one to trace2. But the order of initialization is wrong: va_list ap, cp; va_copy(cp, ap); va_start(ap, fmt); We copy the contents of "ap" into "cp" before it is initialized, meaning it is full of garbage. The two should be swapped. However, there's another bug, noticed by Johannes Schindelin: we forget to call va_end() for the copy. So instead of just fixing the copy's initialization, let's do two separate start/end pairs. This is allowed by the standard, and we don't need to use copy here since we have access to the original varargs. Matching the pairs with the calls makes it more obvious that everything is being done correctly. Note that we do call bug_fl() in the tests, but it didn't trigger this problem because our format string doesn't have any placeholders. So even though we were passing a garbage va_list through the stack, nobody ever needed to look at it. We can easily adjust one of the trace2 tests to trigger this, both for bug() and for BUG(). The latter isn't broken, but it's nice to exercise both a bit more. Without the fix in this patch (but with the test change), the bug() case causes a segfault. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

usage.c: add a non-fatal bug() function to go with BUG()

2022-06-02T19:51:35Z

Add a bug() function to use in cases where we'd like to indicate a runtime BUG(), but would like to defer the BUG() call because we're possibly accumulating more bug() callers to exhaustively indicate what went wrong. We already have this sort of facility in various parts of the codebase, just in the form of ad-hoc re-inventions of the functionality that this new API provides. E.g. this will be used to replace optbug() in parse-options.c, and the 'error("BUG:[...]' we do in a loop in builtin/receive-pack.c. Unlike the code this replaces we'll log to trace2 with this new bug() function (as with other usage.c functions, including BUG()), we'll also be able to avoid calls to xstrfmt() in some cases, as the bug() function itself accepts variadic sprintf()-like arguments. Any caller to bug() can follow up such calls with BUG_if_bug(), which will BUG() out (i.e. abort()) if there were any preceding calls to bug(), callers can also decide not to call BUG_if_bug() and leave the resulting BUG() invocation until exit() time. There are currently no bug() API users that don't call BUG_if_bug() themselves after a for-loop, but allowing for not calling BUG_if_bug() keeps the API flexible. As the tests and documentation here show we'll catch missing BUG_if_bug() invocations in our exit() wrapper. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

C99: remove hardcoded-out !HAVE_VARIADIC_MACROS code

2022-02-22T03:14:19Z

Remove the "else" branches of the HAVE_VARIADIC_MACROS macro, which have been unconditionally omitted since 765dc168882 (git-compat-util: always enable variadic macros, 2021-01-28). Since were always omitted, anyone trying to use a compiler without variadic macro support to compile a git since version git v2.31.0 or later would have had a compilation error. 10 months across a few releases since then should have been enough time for anyone who cared to run into that and report the issue. In addition to that, for anyone unsetting HAVE_VARIADIC_MACROS we've been emitting extremely verbose warnings since at least ee4512ed481 (trace2: create new combined trace facility, 2019-02-22). That's because there is no such thing as a "region_enter_printf" or "region_leave_printf" format, so at least under GCC and Clang everything that includes trace.h (almost every file) emits a couple of warnings about that. There's a large benefit to being able to have a hard dependency rely on variadic macros, the code surrounding usage.c is hard to maintain if we need to write two implementations of everything, and by relying on "__FILE__" and "__LINE__" along with "__VA_ARGS__" we can in the future make error(), die() etc. log where they were called from. We've also recently merged d67fc4bf0ba (Merge branch 'bc/require-c99', 2021-12-10) which further cements our hard dependency on C99. So let's delete the fallback code, and update our CodingGuidelines to note that we depend on this. The added bullet-point starts with lower-case for consistency with other bullet-points in that section. The diff in "trace.h" is relatively hard to read, since we need to retain the existing API docs, which were comments on the code used if HAVE_VARIADIC_MACROS was not defined. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

config API: use get_error_routine(), not vreportf()

2021-12-07T21:25:16Z

Change the git_die_config() function added in 5a80e97c827 (config: add `git_die_config()` to the config-set API, 2014-08-07) to use the public callbacks in the usage.[ch] API instead of the the underlying vreportf() function. In preceding commits the rest of the vreportf() users outside of usage.c was migrated to die_message(), so we can now make it "static". Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano