summaryrefslogtreecommitdiff
path: root/usage.c
AgeCommit message (Collapse)AuthorFilesLines
2021-04-13usage.c: don't copy/paste the same comment three timesLibravatar Ævar Arnfjörð Bjarmason1-12/+5
In ee4512ed481 (trace2: create new combined trace facility, 2019-02-22) we started with two copies of this comment, 0ee10fd1296 (usage: add trace2 entry upon warning(), 2020-11-23) added a third. Let's instead add an earlier comment that applies to all these mostly-the-same functions. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-09usage: trace2 BUG() invocationsLibravatar Jonathan Tan1-0/+11
die() messages are traced in trace2, but BUG() messages are not. Anyone tracking die() messages would have even more reason to track BUG(). Therefore, write to trace2 when BUG() is invoked. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-08Merge branch 'jt/trace-error-on-warning'Libravatar Junio C Hamano1-0/+6
Like die() and error(), a call to warning() will also trigger a trace2 event. * jt/trace-error-on-warning: usage: add trace2 entry upon warning()
2020-11-24usage: add trace2 entry upon warning()Libravatar Jonathan Tan1-0/+6
Emit a trace2 error event whenever warning() is called, just like when die(), error(), or usage() is called. This helps debugging issues that would trigger warnings but not errors. In particular, this might have helped debugging an issue I encountered with commit graphs at $DAYJOB [1]. There is a tradeoff between including potentially relevant messages and cluttering up the trace output produced. I think that warning() messages should be included in traces, because by its nature, Git is used over multiple invocations of the Git tool, and a failure (currently traced) in a Git invocation might be caused by an unexpected interaction in a previous Git invocation that only has a warning (currently untraced) as a symptom - as is the case in [1]. [1] https://lore.kernel.org/git/20200629220744.1054093-1-jonathantanmy@google.com/ Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-16usage: define a type for a reporting functionLibravatar Jeff King1-9/+9
The usage, die, warning, and error routines all work with a function pointer that takes the message to be reported. We usually just mention the function's full type inline. But this makes the use of these pointers hard to read, especially because C's syntax for returning a function pointer is so awful: void (*get_error_routine(void))(const char *err, va_list params); Unless you read it very carefully, this looks like a function pointer declaration. Let's instead use a single typedef to define a reporting function, which is the same for all four types. Note that this also removes the "extern" from these declarations to match the surrounding functions. They were missed in 554544276a (*.[ch]: remove extern from function declarations using spatch, 2019-04-29) presumably because of the unusual syntax. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-02vreportf(): avoid relying on stdio bufferingLibravatar Johannes Schindelin1-4/+16
The MSVC runtime behavior differs from glibc's with respect to `fprintf(stderr, ...)` in that the former writes out the message character by character. In t5516, this leads to a funny problem where a `git fetch` process as well as the `git upload-pack` process spawned by it _both_ call `die()` at the same time. The output can look like this: fatal: git uploadfata-lp: raemcokte :error: upload-pnot our arcef k6: n4ot our ea4cr1e3f 36d45ea94fca1398e86a771eda009872d63adb28598f6a9 8e86a771eda009872d6ab2886 Let's avoid this predicament altogether by rendering the entire message, including the prefix and the trailing newline, into the buffer we already have (and which is still fixed size) and then write it out via `write_in_full()`. We still clip the message to at most 4095 characters. The history of `vreportf()` with regard to this issue includes the following commits: d048a96e (2007-11-09) - 'char msg[256]' is introduced to avoid interleaving 389d1767 (2009-03-25) - Buffer size increased to 1024 to avoid truncation 625a860c (2009-11-22) - Buffer size increased to 4096 to avoid truncation f4c3edc0 (2015-08-11) - Buffer removed to avoid truncation b5a9e435 (2017-01-11) - Reverts f4c3edc0 to be able to replace control chars before sending to stderr 9ac13ec9 (2006-10-11) - Another attempt to solve interleaving. This is seemingly related to d048a96e. 137a0d0e (2007-11-19) - Addresses out-of-order for display() 34df8aba (2009-03-10) - Switches xwrite() to fprintf() in recv_sideband() to support UTF-8 emulation eac14f89 (2012-01-14) - Removes the need for fprintf() for UTF-8 emulation, so it's safe to use xwrite() again 5e5be9e2 (2016-06-28) - recv_sideband() uses xwrite() again Note that we print nothing if the `vsnprintf()` call failed to render the error message; There is little we can do in that case, and it should not happen anyway. The process may have written to `stderr` and there may be something left in the buffer kept in the stdio layer. Call `fflush(stderr)` before writing the message we prepare in this function. Helped-by: Jeff King <peff@peff.net> Helped-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> Helped-by: SZEDER Gábor <szeder.dev@gmail.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22trace2: create new combined trace facilityLibravatar Jeff Hostetler1-0/+31
Create a new unified tracing facility for git. The eventual intent is to replace the current trace_printf* and trace_performance* routines with a unified set of git_trace2* routines. In addition to the usual printf-style API, trace2 provides higer-level event verbs with fixed-fields allowing structured data to be written. This makes post-processing and analysis easier for external tools. Trace2 defines 3 output targets. These are set using the environment variables "GIT_TR2", "GIT_TR2_PERF", and "GIT_TR2_EVENT". These may be set to "1" or to an absolute pathname (just like the current GIT_TRACE). * GIT_TR2 is intended to be a replacement for GIT_TRACE and logs command summary data. * GIT_TR2_PERF is intended as a replacement for GIT_TRACE_PERFORMANCE. It extends the output with columns for the command process, thread, repo, absolute and relative elapsed times. It reports events for child process start/stop, thread start/stop, and per-thread function nesting. * GIT_TR2_EVENT is a new structured format. It writes event data as a series of JSON records. Calls to trace2 functions log to any of the 3 output targets enabled without the need to call different trace_printf* or trace_performance* routines. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-30Merge branch 'jk/snprintf-truncation'Libravatar Junio C Hamano1-0/+1
Avoid unchecked snprintf() to make future code auditing easier. * jk/snprintf-truncation: fmt_with_err: add a comment that truncation is OK shorten_unambiguous_ref: use xsnprintf fsmonitor: use internal argv_array of struct child_process log_write_email_headers: use strbufs http: use strbufs instead of fixed buffers
2018-05-21fmt_with_err: add a comment that truncation is OKLibravatar Jeff King1-0/+1
Functions like die_errno() use fmt_with_err() to combine the caller-provided format with the strerror() string. We use a fixed stack buffer because we're already handling an error and don't have any way to report another one. Our buffer should generally be big enough to fit this, but if it's not, truncation is our best option. Let's add a comment to that effect, so that anybody auditing the code for truncation bugs knows that this is fine. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-06test-tool: help verifying BUG() code pathsLibravatar Johannes Schindelin1-0/+5
When we call BUG(), we signal via SIGABRT that something bad happened, dumping cores if so configured. In some setups these coredumps are redirected to some central place such as /proc/sys/kernel/core_pattern, which is a good thing. However, when we try to verify in our test suite that bugs are caught in certain code paths, we do *not* want to clutter such a central place with unnecessary coredumps. So let's special-case the test helpers (which we use to verify such code paths) so that the BUG() calls will *not* call abort() but exit with a special-purpose exit code instead. Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-08add UNLEAK annotation for reducing leak false positivesLibravatar Jeff King1-0/+15
It's a common pattern in git commands to allocate some memory that should last for the lifetime of the program and then not bother to free it, relying on the OS to throw it away. This keeps the code simple, and it's fast (we don't waste time traversing structures or calling free at the end of the program). But it also triggers warnings from memory-leak checkers like valgrind or LSAN. They know that the memory was still allocated at program exit, but they don't know _when_ the leaked memory stopped being useful. If it was early in the program, then it's probably a real and important leak. But if it was used right up until program exit, it's not an interesting leak and we'd like to suppress it so that we can see the real leaks. This patch introduces an UNLEAK() macro that lets us do so. To understand its design, let's first look at some of the alternatives. Unfortunately the suppression systems offered by leak-checking tools don't quite do what we want. A leak-checker basically knows two things: 1. Which blocks were allocated via malloc, and the callstack during the allocation. 2. Which blocks were left un-freed at the end of the program (and which are unreachable, but more on that later). Their suppressions work by mentioning the function or callstack of a particular allocation, and marking it as OK to leak. So imagine you have code like this: int cmd_foo(...) { /* this allocates some memory */ char *p = some_function(); printf("%s", p); return 0; } You can say "ignore allocations from some_function(), they're not leaks". But that's not right. That function may be called elsewhere, too, and we would potentially want to know about those leaks. So you can say "ignore the callstack when main calls some_function". That works, but your annotations are brittle. In this case it's only two functions, but you can imagine that the actual allocation is much deeper. If any of the intermediate code changes, you have to update the suppression. What we _really_ want to say is that "the value assigned to p at the end of the function is not a real leak". But leak-checkers can't understand that; they don't know about "p" in the first place. However, we can do something a little bit tricky if we make some assumptions about how leak-checkers work. They generally don't just report all un-freed blocks. That would report even globals which are still accessible when the leak-check is run. Instead they take some set of memory (like BSS) as a root and mark it as "reachable". Then they scan the reachable blocks for anything that looks like a pointer to a malloc'd block, and consider that block reachable. And then they scan those blocks, and so on, transitively marking anything reachable from a global as "not leaked" (or at least leaked in a different category). So we can mark the value of "p" as reachable by putting it into a variable with program lifetime. One way to do that is to just mark "p" as static. But that actually affects the run-time behavior if the function is called twice (you aren't likely to call main() twice, but some of our cmd_*() functions are called from other commands). Instead, we can trick the leak-checker by putting the value into _any_ reachable bytes. This patch keeps a global linked-list of bytes copied from "unleaked" variables. That list is reachable even at program exit, which confers recursive reachability on whatever values we unleak. In other words, you can do: int cmd_foo(...) { char *p = some_function(); printf("%s", p); UNLEAK(p); return 0; } to annotate "p" and suppress the leak report. But wait, couldn't we just say "free(p)"? In this toy example, yes. But UNLEAK()'s byte-copying strategy has several advantages over actually freeing the memory: 1. It's recursive across structures. In many cases our "p" is not just a pointer, but a complex struct whose fields may have been allocated by a sub-function. And in some cases (e.g., dir_struct) we don't even have a function which knows how to free all of the struct members. By marking the struct itself as reachable, that confers reachability on any pointers it contains (including those found in embedded structs, or reachable by walking heap blocks recursively. 2. It works on cases where we're not sure if the value is allocated or not. For example: char *p = argc > 1 ? argv[1] : some_function(); It's safe to use UNLEAK(p) here, because it's not freeing any memory. In the case that we're pointing to argv here, the reachability checker will just ignore our bytes. 3. Likewise, it works even if the variable has _already_ been freed. We're just copying the pointer bytes. If the block has been freed, the leak-checker will skip over those bytes as uninteresting. 4. Because it's not actually freeing memory, you can UNLEAK() before we are finished accessing the variable. This is helpful in cases like this: char *p = some_function(); return another_function(p); Writing this with free() requires: int ret; char *p = some_function(); ret = another_function(p); free(p); return ret; But with unleak we can just write: char *p = some_function(); UNLEAK(p); return another_function(p); This patch adds the UNLEAK() macro and enables it automatically when Git is compiled with SANITIZE=leak. In normal builds it's a noop, so we pay no runtime cost. It also adds some UNLEAK() annotations to show off how the feature works. On top of other recent leak fixes, these are enough to get t0000 and t0001 to pass when compiled with LSAN. Note the case in commit.c which actually converts a strbuf_release() into an UNLEAK. This code was already non-leaky, but the free didn't do anything useful, since we're exiting. Converting it to an annotation means that non-leak-checking builds pay no runtime cost. The cost is minimal enough that it's probably not worth going on a crusade to convert these kinds of frees to UNLEAKS. I did it here for consistency with the "sb" leak (though it would have been equally correct to go the other way, and turn them both into strbuf_release() calls). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-21die(): stop hiding errors due to overzealous recursion guardLibravatar Ævar Arnfjörð Bjarmason1-1/+17
Change the recursion limit for the default die routine from a *very* low 1 to 1024. This ensures that infinite recursions are broken, but doesn't lose the meaningful error messages under threaded execution where threads concurrently start to die. The intent of the existing code, as explained in commit cd163d4b4e ("usage.c: detect recursion in die routines and bail out immediately", 2012-11-14), is to break infinite recursion in cases where the die routine itself calls die(), and would thus infinitely recurse. However, doing that very aggressively by immediately printing out "recursion detected in die handler" if we've already called die() once means that threaded invocations of git can end up only printing out the "recursion detected" error, while hiding the meaningful error. An example of this is running a threaded grep which dies on execution against pretty much any repo, git.git will do: git grep -P --threads=8 '(*LIMIT_MATCH=1)-?-?-?---$' With the current version of git this will print some combination of multiple PCRE failures that caused the abort and multiple "recursion detected", some invocations will print out multiple "recursion detected" errors with no PCRE error at all! Before this change, running the above grep command 1000 times against git.git[1] and taking the top 20 results will on my system yield the following distribution of actual errors ("E") and recursion errors ("R"): 322 E R 306 E 116 E R R 65 R R 54 R E 49 E E 44 R 15 E R R R 9 R R R 7 R E R 5 R R E 3 E R R R R 2 E E R 1 R R R R 1 R R R E 1 R E R R The exact results are obviously random and system-dependent, but this shows the race condition in this code. Some small part of the time we're about to print out the actual error ("E") but another thread's recursion error beats us to it, and sometimes we print out nothing but the recursion error. With this change we get, now with "W" to mean the new warning being emitted indicating that we've called die() many times: 502 E 160 E W E 120 E E 53 E W 35 E W E E 34 W E E 29 W E E E 16 E E W 16 E E E 11 W E E E E 7 E E W E 4 W E 3 W W E E 2 E W E E E 1 W W E 1 W E W E 1 E W W E E E 1 E W W E E 1 E W W E 1 E W E E W Which still sucks a bit, due to a still present race-condition in this code we're sometimes going to print out several errors still, or several warnings, or two duplicate errors without the warning. But we will never have a case where we completely hide the actual error as we do now. Now, git-grep could make use of the pluggable error facility added in commit c19a490e37 ("usage: allow pluggable die-recursion checks", 2013-04-16). There's other threaded code that calls set_die_routine() or set_die_is_recursing_routine(). But this is about fixing the general die() behavior with threading when we don't have such a custom routine yet. Right now the common case is not an infinite recursion in the handler, but us losing error messages by default because we're overly paranoid about our recursion check. So let's just set the recursion limit to a number higher than the number of threads we're ever likely to spawn. Now we won't lose errors, and if we have a recursing die handler we'll still die within microseconds. There are race conditions in this code itself, in particular the "dying" variable is not thread mutexed, so we e.g. won't be dying at exactly 1024, or for that matter even be able to accurately test "dying == 2", see the cases where we print out more than one "W" above. But that doesn't really matter, for the recursion guard we just need to die "soon", not at exactly 1024 calls, and for printing the correct error and only one warning most of the time in the face of threaded death this is good enough and a net improvement on the current code. 1. for i in {1..1000}; do git grep -P --threads=8 '(*LIMIT_MATCH=1)-?-?-?---$' 2>&1|perl -pe 's/^fatal: r.*/R/; s/^fatal: p.*/E/; s/^warning.*/W/' | tr '\n' ' '; echo; done | sort | uniq -c | sort -nr | head -n 20 Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-13Merge branch 'bw/forking-and-threading' into maintLibravatar Junio C Hamano1-9/+1
The "run-command" API implementation has been made more robust against dead-locking in a threaded environment. * bw/forking-and-threading: usage.c: drop set_error_handle() run-command: restrict PATH search to executable files run-command: expose is_executable function run-command: block signals between fork and execve run-command: add note about forking and threading run-command: handle dup2 and close errors in child run-command: eliminate calls to error handling functions in child run-command: don't die in child when duping /dev/null run-command: prepare child environment before forking string-list: add string_list_remove function run-command: use the async-signal-safe execv instead of execvp run-command: prepare command before forking t0061: run_command executes scripts without a #! line t5550: use write_script to generate post-update hook
2017-05-22usage: add NORETURN to BUG() function definitionsLibravatar Ramsay Jones1-2/+2
Commit d8193743e0 ("usage.c: add BUG() function", 12-05-2017) added the BUG() functions and macros as a replacement for calls to die("BUG: .."). The use of NORETURN on the declarations (in git-compat-util.h) and the lack of NORETURN on the function definitions, however, leads sparse to complain thus: SP usage.c usage.c:220:6: error: symbol 'BUG_fl' redeclared with different type (originally declared at git-compat-util.h:1074) - different modifiers In order to suppress the sparse error, add the NORETURN to the function definitions. Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-15usage.c: drop set_error_handle()Libravatar Jeff King1-9/+1
The set_error_handle() function was introduced by 3b331e926 (vreportf: report to arbitrary filehandles, 2015-08-11) so that run-command could send post-fork, pre-exec errors to the parent's original stderr. That use went away in 79319b194 (run-command: eliminate calls to error handling functions in child, 2017-04-19), which pushes all of the error reporting to the parent. This leaves no callers of set_error_handle(). As we're not likely to add any new ones, let's drop it. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Brandon Williams <bmwill@google.com> Reviewed-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-15usage.c: add BUG() functionLibravatar Jeff King1-0/+32
There's a convention in Git's code base to write assertions as: if (...some_bad_thing...) die("BUG: the terrible thing happened"); with the idea that users should never see a "BUG:" message (but if they, it at least gives a clue what happened). We use die() here because it's convenient, but there are a few draw-backs: 1. Without parsing the messages, it's hard for callers to distinguish BUG assertions from regular errors. For instance, it would be nice if the test suite could check that we don't hit any assertions, but test_must_fail will pass BUG deaths as OK. 2. It would be useful to add more debugging features to BUG assertions, like file/line numbers or dumping core. 3. The die() handler can be replaced, and might not actually exit the whole program (e.g., it may just pthread_exit()). This is convenient for normal errors, but for an assertion failure (which is supposed to never happen), we're probably better off taking down the whole process as quickly and cleanly as possible. We could address these by checking in die() whether the error message starts with "BUG", and behaving appropriately. But there's little advantage at that point to sharing the die() code, and only downsides (e.g., we can't change the BUG() interface independently). Moreover, converting all of the existing BUG calls reveals that the test suite does indeed trigger a few of them. Instead, this patch introduces a new BUG() function, which prints an error before dying via SIGABRT. This gives us test suite checking and core dumps. The function is actually a macro (when supported) so that we can show the file/line number. We can convert die("BUG") invocations to BUG() in further patches, dealing with any test fallouts individually. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-31Merge branch 'jk/vreport-sanitize'Libravatar Junio C Hamano1-10/+7
An error message with an ASCII control character like '\r' in it can alter the message to hide its early part, which is problematic when a remote side gives such an error message that the local side will relay with a "remote: " prefix. * jk/vreport-sanitize: vreport: sanitize ASCII control chars Revert "vreportf: avoid intermediate buffer"
2017-01-11vreport: sanitize ASCII control charsLibravatar Jeff King1-0/+6
Our error() and die() calls may report messages with arbitrary data (e.g., filenames or even data from a remote server). Let's make it harder to cause confusion with mischievous filenames. E.g., try: git rev-parse "$(printf "\rfatal: this argument is too sneaky")" -- or git rev-parse "$(printf "\x1b[5mblinky\x1b[0m")" -- Let's block all ASCII control characters, with the exception of TAB and LF. We use both in our own messages (and we are necessarily sanitizing the complete output of snprintf here, as we do not have access to the individual varargs). And TAB and LF are unlikely to cause confusion (you could put "\nfatal: sneaky\n" in your filename, but it would at least not _cover up_ the message leading to it, unlike "\r"). We'll replace the characters with a "?", which is similar to how "ls" behaves. It might be nice to do something less lossy, like converting them to "\x" hex codes. But replacing with a single character makes it easy to do in-place and without worrying about length limitations. This feature should kick in rarely enough that the "?" marks are almost never seen. We'll leave high-bit characters as-is, as they are likely to be UTF-8 (though there may be some Unicode mischief you could cause, which may require further patches). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-11Revert "vreportf: avoid intermediate buffer"Libravatar Jeff King1-12/+3
This reverts commit f4c3edc0b156362a92bf9de4f0ec794e90a757fc. The purpose of that commit was to let us write errors of arbitrary length to stderr by skipping the intermediate buffer and sending our varargs straight to fprintf. That works, but it comes with a downside: we do not get access to the varargs before they are sent to stderr. On balance, it's not a good tradeoff. Error messages larger than our 4K buffer are quite uncommon, and we've lost the ability to make any modifications to the output (e.g., to remove non-printable characters). The only way to have both would be one of: 1. Write into a dynamic buffer. But this is a bad idea for a low-level function that may be called when malloc() has failed. 2. Do our own printf-format varargs parsing. This is too complex to be worth the trouble. Let's just revert that change and go back to a fixed buffer. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-19Merge branch 'cc/apply-am'Libravatar Junio C Hamano1-0/+15
"git am" has been taught to make an internal call to "git apply"'s innards without spawning the latter as a separate process. * cc/apply-am: (41 commits) builtin/am: use apply API in run_apply() apply: learn to use a different index file apply: pass apply state to build_fake_ancestor() apply: refactor `git apply` option parsing apply: change error_routine when silent usage: add get_error_routine() and get_warn_routine() usage: add set_warn_routine() apply: don't print on stdout in verbosity_silent mode apply: make it possible to silently apply apply: use error_errno() where possible apply: make some parsing functions static again apply: move libified code from builtin/apply.c to apply.{c,h} apply: rename and move opt constants to apply.h builtin/apply: rename option parsing functions builtin/apply: make create_one_file() return -1 on error builtin/apply: make try_create_file() return -1 on error builtin/apply: make write_out_results() return -1 on error builtin/apply: make write_out_one_result() return -1 on error builtin/apply: make create_file() return -1 on error builtin/apply: make add_index_file() return -1 on error ...
2016-09-07usage: add get_error_routine() and get_warn_routine()Libravatar Christian Couder1-0/+10
Let's make it possible to get the current error_routine and warn_routine, so that we can store them before using set_error_routine() or set_warn_routine() to use new ones. This way we will be able put back the original routines, when we are done with using new ones. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-07usage: add set_warn_routine()Libravatar Christian Couder1-0/+5
There are already set_die_routine() and set_error_routine(), so let's add set_warn_routine() as this will be needed in a following commit. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-31error_errno: use constant return similar to error()Libravatar Jeff King1-0/+1
Commit e208f9c (make error()'s constant return value more visible, 2012-12-15) introduced some macro trickery to make the constant return from error() more visible to callers, which in turn can help gcc produce better warnings (and possibly even better code). Later, fd1d672 (usage.c: add warning_errno() and error_errno(), 2016-05-08) introduced another variant, and subsequent commits converted some uses of error() to error_errno(), losing the magic from e208f9c for those sites. As a result, compiling vcs-svn/svndiff.c with "gcc -O3" produces -Wmaybe-uninitialized false positives (at least with gcc 6.2.0). Let's give error_errno() the same treatment, which silences these warnings. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-09usage.c: add warning_errno() and error_errno()Libravatar Nguyễn Thái Ngọc Duy1-0/+21
Similar to die_errno(), these functions will append strerror() automatically. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-09usage.c: move format processing out of die_errno()Libravatar Nguyễn Thái Ngọc Duy1-11/+16
fmt_with_err() will be shared with the coming error_errno() and warning_errno(). Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-11vreportf: avoid intermediate bufferLibravatar Jeff King1-3/+12
When we call "die(fmt, args...)", we end up in vreportf with two pieces of information: 1. The prefix "fatal: " 2. The original fmt and va_list of args. We format item (2) into a temporary buffer, and then fprintf the prefix and the temporary buffer, along with a newline. This has the unfortunate side effect of truncating any error messages that are longer than 4096 bytes. Instead, let's use separate calls for the prefix and newline, letting us hand the item (2) directly to vfprintf. This is essentially undoing d048a96 (print warning/error/fatal messages in one shot, 2007-11-09), which tried to have the whole output end up in a single `write` call. But we can address this instead by explicitly requesting line-buffering for the output handle, and by making sure that the buffer is empty before we start (so that outputting the prefix does not cause a flush due to hitting the buffer limit). We may still break the output into two writes if the content is larger than our buffer, but there's not much we can do there; depending on the stdio implementation, that might have happened even with a single fprintf call. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-11vreportf: report to arbitrary filehandlesLibravatar Jeff King1-13/+9
The vreportf function always goes to stderr, but run-command wants child errors to go to the parent's original stderr. To solve this, commit a5487dd duplicates the stderr fd and installs die and error handlers to direct the output appropriately (which later turned into the vwritef function). This has two downsides, though: - we make multiple calls to write(), which contradicts the "write at once" logic from d048a96 (print warning/error/fatal messages in one shot, 2007-11-09). - the custom handlers basically duplicate the normal handlers. They're only a few lines of code, but we should not have to repeat the magic "exit(128)", for example. We can solve the first by using fdopen() on the duplicated descriptor. We can't pass this to vreportf, but we could introduce a new vreportf_to to handle it. However, to fix the second problem, we instead introduce a new "set_error_handle" function, which lets the normal vreportf calls output to a handle besides stderr. Thus we can get rid of our custom handlers entirely, and just ask the regular handlers to output to our new descriptor. And as vwritef has no more callers, it can just go away. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-19Merge branch 'jk/a-thread-only-dies-once'Libravatar Junio C Hamano1-6/+14
A regression fix for the logic to detect die() handler triggering itself recursively. * jk/a-thread-only-dies-once: run-command: use thread-aware die_is_recursing routine usage: allow pluggable die-recursion checks
2013-04-16usage: allow pluggable die-recursion checksLibravatar Jeff King1-6/+14
When any git code calls die or die_errno, we use a counter to detect recursion into the die functions from any of the helper functions. However, such a simple counter is not good enough for threaded programs, which may call die from a sub-thread, killing only the sub-thread (but incrementing the counter for everyone). Rather than try to deal with threads ourselves here, let's just allow callers to plug in their own recursion-detection function. This is similar to how we handle the die routine (the caller plugs in a die routine which may kill only the sub-thread). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-12-15make error()'s constant return value more visibleLibravatar Jeff King1-0/+1
When git is compiled with "gcc -Wuninitialized -O3", some inlined calls provide an additional opportunity for the compiler to do static analysis on variable initialization. For example, with two functions like this: int get_foo(int *foo) { if (something_that_might_fail() < 0) return error("unable to get foo"); *foo = 0; return 0; } void some_fun(void) { int foo; if (get_foo(&foo) < 0) return -1; printf("foo is %d\n", foo); } If get_foo() is not inlined, then when compiling some_fun, gcc sees only that a pointer to the local variable is passed, and must assume that it is an out parameter that is initialized after get_foo returns. However, when get_foo() is inlined, the compiler may look at all of the code together and see that some code paths in get_foo() do not initialize the variable. As a result, it prints a warning. But what the compiler can't see is that error() always returns -1, and therefore we know that either we return early from some_fun, or foo ends up initialized, and the code is safe. The warning is a false positive. If we can make the compiler aware that error() will always return -1, it can do a better job of analysis. The simplest method would be to inline the error() function. However, this doesn't work, because gcc will not inline a variadc function. We can work around this by defining a macro. This relies on two gcc extensions: 1. Variadic macros (these are present in C99, but we do not rely on that). 2. Gcc treats the "##" paste operator specially between a comma and __VA_ARGS__, which lets our variadic macro work even if no format parameters are passed to error(). Since we are using these extra features, we hide the macro behind an #ifdef. This is OK, though, because our goal was just to help gcc. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-11-15usage.c: detect recursion in die routines and bail out immediatelyLibravatar Brandon Casey1-0/+15
It is theoretically possible for a die handler to get into a state of infinite recursion. For example, if a die handler called another function which itself called die(). Let's at least detect this situation, inform the user, and call exit. Signed-off-by: Brandon Casey <bcasey@nvidia.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-07-31error_routine: use parent's stderr if exec failsLibravatar Clemens Buchacher1-0/+18
The new process's error output may be redirected elsewhere, but if the exec fails, output should still go to the parent's stderr. This has already been done for the die_routine. Do the same for error_routine. Signed-off-by: Clemens Buchacher <drizzd@aon.at> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-03-22Fix sparse warningsLibravatar Stephen Boyd1-4/+4
Fix warnings from 'make check'. - These files don't include 'builtin.h' causing sparse to complain that cmd_* isn't declared: builtin/clone.c:364, builtin/fetch-pack.c:797, builtin/fmt-merge-msg.c:34, builtin/hash-object.c:78, builtin/merge-index.c:69, builtin/merge-recursive.c:22 builtin/merge-tree.c:341, builtin/mktag.c:156, builtin/notes.c:426 builtin/notes.c:822, builtin/pack-redundant.c:596, builtin/pack-refs.c:10, builtin/patch-id.c:60, builtin/patch-id.c:149, builtin/remote.c:1512, builtin/remote-ext.c:240, builtin/remote-fd.c:53, builtin/reset.c:236, builtin/send-pack.c:384, builtin/unpack-file.c:25, builtin/var.c:75 - These files have symbols which should be marked static since they're only file scope: submodule.c:12, diff.c:631, replace_object.c:92, submodule.c:13, submodule.c:14, trace.c:78, transport.c:195, transport-helper.c:79, unpack-trees.c:19, url.c:3, url.c:18, url.c:104, url.c:117, url.c:123, url.c:129, url.c:136, thread-utils.c:21, thread-utils.c:48 - These files redeclare symbols to be different types: builtin/index-pack.c:210, parse-options.c:564, parse-options.c:571, usage.c:49, usage.c:58, usage.c:63, usage.c:72 - These files use a literal integer 0 when they really should use a NULL pointer: daemon.c:663, fast-import.c:2942, imap-send.c:1072, notes-merge.c:362 While we're in the area, clean up some unused #includes in builtin files (mostly exec_cmd.h). Signed-off-by: Stephen Boyd <bebarino@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-03-07Make report() from usage.c public as vreportf() and use it.Libravatar Johannes Sixt1-5/+5
There exist already a number of static functions named 'report', therefore, the function name was changed. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-22Fix truncated usage messagesLibravatar Björn Gustavsson1-1/+1
The usage messages for some commands (such as 'git diff-tree') are truncated because they don't fit in a fixed buffer of 1024 bytes. It would be tempting to eliminate the buffer and the problem once and for all by doing the output in three steps, but doing so could (according to commit d048a96e) increase the likelyhood of messing up the display. So we just increase the size of the buffer. Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-10Introduce usagef() that takes a printf-style formatLibravatar Jonathan Nieder1-4/+13
Some new callers would want to use printf-like formatting, when issuing their usage messages. An option is to change usage() itself also be like printf(), which would make it similar to die() and warn(). But usage() is typically fixed, as opposed to die() and warn() that gives diagnostics depending on the situation. Indeed, the majority of strings given by existing callsites to usage() are fixed strings. If we were to make usage() take printf-style format, they all need to be changed to have "%s" as their first argument. So instead, introduce usagef() so that limited number of callers can use it. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-01add NORETURN_PTR for function pointersLibravatar Erik Faye-Lund1-3/+3
Some compilers (including at least MSVC and ARM RVDS) supports NORETURN on function declarations, but not on function pointers. This patch makes it possible to define NORETURN for these compilers, by splitting the NORETURN macro into two - one for function declarations and one for function pointers. Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Jeff King <peff@peff.net>
2009-10-01increase portability of NORETURN declarationsLibravatar Erik Faye-Lund1-3/+3
Some compilers (including at least MSVC) support NORETURN on function declarations, but only before the function-name. This patch makes it possible to define NORETURN to something meaningful for those compilers. Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Jeff King <peff@peff.net>
2009-06-27die_errno(): double % in strerror() output just in caseLibravatar Junio C Hamano1-2/+18
[tr: handle border case where % is placed at end of buffer] Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Thomas Rast <trast@student.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-06-27Introduce die_errno() that appends strerror(errno) to die()Libravatar Thomas Rast1-0/+12
There are many calls to die() that do, or should, report strerror(errno) to indicate how the syscall they guard failed. Introduce a small helper function for this case. Note: - POSIX says vsnprintf can modify errno in some unlikely cases, so we have to use errno early. - We take some care to pass the original format to die_routine(), in case someone wants to call die_errno() with custom format characters. Signed-off-by: Thomas Rast <trast@student.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-03-24Increase the size of the die/warning buffer to avoid truncationLibravatar Shawn O. Pearce1-1/+1
Long messages like those from lockfile.c when a lock can't be obtained truncate with only 256 bytes in the message buffer. Bump it to 1024 to give more space for these longer cases. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-09-29usage.c: remove unused functionsLibravatar Nanako Shiraishi1-16/+0
This removes three functions that are not used anywhere. Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com> Acked-by: Lars Hjemli <hjemli@gmail.com> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
2007-11-09print warning/error/fatal messages in one shotLibravatar Nicolas Pitre1-3/+3
Not doing so is likely to create a messed up display when sent over the sideband protocol. Signed-off-by: Nicolas Pitre <nico@cam.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2007-03-31Rename warn() to warning() to fix symbol conflicts on BSD and Mac OSLibravatar Theodore Ts'o1-1/+1
This fixes a problem reported by Randal Schwartz: >I finally tracked down all the (albeit inconsequential) errors I was getting >on both OpenBSD and OSX. It's the warn() function in usage.c. There's >warn(3) in BSD-style distros. It'd take a "great rename" to change it, but if >someone with better C skills than I have could do that, my linker and I would >appreciate it. It was annoying to me, too, when I was doing some mergetool testing on Mac OS X, so here's a fix. Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> Cc: "Randal L. Schwartz" <merlyn@stonehenge.com> Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-12-21Introduce a global level warn() function.Libravatar Shawn O. Pearce1-0/+19
Like the existing error() function the new warn() function can be used to describe a situation that probably should not be occuring, but which the user (and Git) can continue to work around without running into too many problems. An example situation is a bad commit SHA1 found in a reflog. Attempting to read this record out of the reflog isn't really an error as we have skipped over it in the past. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-06-24usage: minimum type fix.Libravatar Junio C Hamano1-3/+3
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-06-24Customizable error handlersLibravatar Petr Baudis1-4/+42
This patch makes the usage(), die() and error() handlers customizable. Nothing in the git code itself uses that but many other libgit users (like Git.pm) will. This is implemented using the mutator functions primarily because you cannot directly modifying global variables of libgit from a program that dlopen()ed it, apparently. But having functions for that is a better API anyway. Signed-off-by: Petr Baudis <pasky@suse.cz> Signed-off-by: Junio C Hamano <junkio@cox.net>
2005-12-05Clean up compatibility definitions.Libravatar Junio C Hamano1-1/+1
This attempts to clean up the way various compatibility functions are defined and used. - A new header file, git-compat-util.h, is introduced. This looks at various NO_XXX and does necessary function name replacements, equivalent of -Dstrcasestr=gitstrcasestr in the Makefile. - Those function name replacements are removed from the Makefile. - Common features such as usage(), die(), xmalloc() are moved from cache.h to git-compat-util.h; cache.h includes git-compat-util.h itself. Signed-off-by: Junio C Hamano <junkio@cox.net>
2005-10-01[PATCH] Better error reporting for "git status"Libravatar Linus Torvalds1-2/+2
Instead of "git status" ignoring (and hiding) potential errors from the "git-update-index" call, make it exit if it fails, and show the error. In order to do this, use the "-q" flag (to ignore not-up-to-date files) and add a new "--unmerged" flag that allows unmerged entries in the index without any errors. This also avoids marking the index "changed" if an entry isn't actually modified, and makes sure that we exit with an understandable error message if the index is corrupt or unreadable. "read_cache()" no longer returns an error for the caller to check. Finally, make die() and usage() exit with recognizable error codes, if we ever want to check the failure reason in scripts. Signed-off-by: Linus Torvalds <torvalds@osdl.org> Signed-off-by: Junio C Hamano <junkio@cox.net>
2005-05-22Include file cleanups..Libravatar Linus Torvalds1-1/+0
Add <limits.h> to the include files handled by "cache.h", and remove extraneous #include directives from various .c files. The rule is that "cache.h" gets all the basic stuff, so that we'll have as few system dependencies as possible.