From 896671d4c74d6e72675564a83d140c19d7d937bd Mon Sep 17 00:00:00 2001 From: Josh Steadmon Date: Thu, 25 Apr 2019 10:08:53 -0700 Subject: [PATCH 1/4] trace2: fix incorrect function pointer check Fix trace2_data_json_fl() to check for the presence of pfn_data_json_fl in its targets, rather than pfn_data_fl, which is not actually called. Signed-off-by: Josh Steadmon Signed-off-by: Junio C Hamano --- trace2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trace2.c b/trace2.c index 6baa65cdf9aee6..60337f570ad865 100644 --- a/trace2.c +++ b/trace2.c @@ -727,7 +727,7 @@ void trace2_data_json_fl(const char *file, int line, const char *category, us_elapsed_region = tr2tls_region_elasped_self(us_now); for_each_wanted_builtin (j, tgt_j) - if (tgt_j->pfn_data_fl) + if (tgt_j->pfn_data_json_fl) tgt_j->pfn_data_json_fl(file, line, us_elapsed_absolute, us_elapsed_region, category, repo, key, value); From 0ae7a0bd17e6db8d6aab63aca2598f55c76278d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 10 May 2019 15:37:38 +0200 Subject: [PATCH 2/4] trace2: fix up a missing "leave" entry point MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a trivial bug that's been here since the shared/do_write_index tracing was added in 42fee7a388 ("trace2:data: add trace2 instrumentation to index read/write", 2019-02-22). We should have enter/leave points, not two enter/enter points. This resulted in an "enter" event without a corresponding "leave" event. Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- read-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/read-cache.c b/read-cache.c index a880aaedffbc5e..8c3860986368b2 100644 --- a/read-cache.c +++ b/read-cache.c @@ -3146,7 +3146,7 @@ static int write_shared_index(struct index_state *istate, trace2_region_enter_printf("index", "shared/do_write_index", the_repository, "%s", (*temp)->filename.buf); ret = do_write_index(si->base, *temp, 1); - trace2_region_enter_printf("index", "shared/do_write_index", + trace2_region_leave_printf("index", "shared/do_write_index", the_repository, "%s", (*temp)->filename.buf); if (ret) From e8e6ea95116e70bf854463a8ab72687a9e821e56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torsten=20B=C3=B6gershausen?= Date: Tue, 19 Mar 2019 17:13:46 +0000 Subject: [PATCH 3/4] trace2: NULL is not allowed for va_list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some compilers don't allow NULL to be passed for a va_list, and e.g. "gcc (Raspbian 6.3.0-18+rpi1+deb9u1) 6.3.0 20170516" errors out like this: trace2/tr2_tgt_event.c:193:18: error: invalid operands to binary && (have ‘int’ and ‘va_list {aka __va_list}’) if (fmt && *fmt && ap) { ^^ I couldn't find any hints that va_list and pointers can be mixed, and no hints that they can't either. Morten Welinder comments: "C99, Section 7.15, simply says that va_list "is an object type suitable for holding information needed by the macros va_start, va_end, and va_copy". So clearly not guaranteed to be mixable with pointers... The portable solution is to use "va_list" everywhere in the callchain. As a consequence, both trace2_region_enter_fl() and trace2_region_leave_fl() now take a variable argument list. Signed-off-by: Torsten Bögershausen Acked-by: Jeff Hostetler Signed-off-by: Junio C Hamano --- trace2.c | 15 +++++++++++---- trace2.h | 4 ++-- trace2/tr2_tgt_event.c | 2 +- trace2/tr2_tgt_normal.c | 2 +- trace2/tr2_tgt_perf.c | 2 +- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/trace2.c b/trace2.c index 60337f570ad865..c7b4f14d29a9d0 100644 --- a/trace2.c +++ b/trace2.c @@ -565,10 +565,14 @@ void trace2_region_enter_printf_va_fl(const char *file, int line, } void trace2_region_enter_fl(const char *file, int line, const char *category, - const char *label, const struct repository *repo) + const char *label, const struct repository *repo, ...) { + va_list ap; + va_start(ap, repo); trace2_region_enter_printf_va_fl(file, line, category, label, repo, - NULL, NULL); + NULL, ap); + va_end(ap); + } void trace2_region_enter_printf_fl(const char *file, int line, @@ -638,10 +642,13 @@ void trace2_region_leave_printf_va_fl(const char *file, int line, } void trace2_region_leave_fl(const char *file, int line, const char *category, - const char *label, const struct repository *repo) + const char *label, const struct repository *repo, ...) { + va_list ap; + va_start(ap, repo); trace2_region_leave_printf_va_fl(file, line, category, label, repo, - NULL, NULL); + NULL, ap); + va_end(ap); } void trace2_region_leave_printf_fl(const char *file, int line, diff --git a/trace2.h b/trace2.h index 888531eb08380c..f189ef5984f0e2 100644 --- a/trace2.h +++ b/trace2.h @@ -257,7 +257,7 @@ void trace2_def_repo_fl(const char *file, int line, struct repository *repo); * on this thread. */ void trace2_region_enter_fl(const char *file, int line, const char *category, - const char *label, const struct repository *repo); + const char *label, const struct repository *repo, ...); #define trace2_region_enter(category, label, repo) \ trace2_region_enter_fl(__FILE__, __LINE__, (category), (label), (repo)) @@ -297,7 +297,7 @@ void trace2_region_enter_printf(const char *category, const char *label, * in this nesting level. */ void trace2_region_leave_fl(const char *file, int line, const char *category, - const char *label, const struct repository *repo); + const char *label, const struct repository *repo, ...); #define trace2_region_leave(category, label, repo) \ trace2_region_leave_fl(__FILE__, __LINE__, (category), (label), (repo)) diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c index 2c97cf54be95e2..c2852d1bd2bd85 100644 --- a/trace2/tr2_tgt_event.c +++ b/trace2/tr2_tgt_event.c @@ -193,7 +193,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code) static void maybe_add_string_va(struct json_writer *jw, const char *field_name, const char *fmt, va_list ap) { - if (fmt && *fmt && ap) { + if (fmt && *fmt) { va_list copy_ap; struct strbuf buf = STRBUF_INIT; diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c index 1ce6f978631930..00b116d797c844 100644 --- a/trace2/tr2_tgt_normal.c +++ b/trace2/tr2_tgt_normal.c @@ -127,7 +127,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code) static void maybe_append_string_va(struct strbuf *buf, const char *fmt, va_list ap) { - if (fmt && *fmt && ap) { + if (fmt && *fmt) { va_list copy_ap; va_copy(copy_ap, ap); diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c index 328d2234bddf04..ea0cbbe13ee066 100644 --- a/trace2/tr2_tgt_perf.c +++ b/trace2/tr2_tgt_perf.c @@ -212,7 +212,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code) static void maybe_append_string_va(struct strbuf *buf, const char *fmt, va_list ap) { - if (fmt && *fmt && ap) { + if (fmt && *fmt) { va_list copy_ap; va_copy(copy_ap, ap); From 986312dc2ab9fb5227515d8286ca135e9aed3304 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Mon, 29 Apr 2019 13:14:22 -0700 Subject: [PATCH 4/4] trace2: fixup access problem on /etc/gitconfig in read_very_early_config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NOTE: This commit was modified when merged into the vfs-2.21 series NOTE: to account for programdata config in addition to etc/gitconfig. Teach do_git_config_sequence() to optionally gently check for access to the system config. Use this option in read_very_early_config() when initializing trace2. In [1] SZEDER Gábor reported that my changes in [2] introduced a regression when the user does not have permission to read the system config. This commit addresses that problem by optionally ignoring that error. [1] https://public-inbox.org/git/285beb2b2d740ce20fdd8af1becf371ab39703db.1554995916.git.gitgitgadget@gmail.com/T/#m342e839289aec515523a98b5e34d7f42d3f1fd79 [2] https://public-inbox.org/git/285beb2b2d740ce20fdd8af1becf371ab39703db.1554995916.git.gitgitgadget@gmail.com/T/#m11b59c9228c698442f750ee8f9b10c629399ae48 Signed-off-by: Jeff Hostetler Signed-off-by: Junio C Hamano --- config.c | 7 +++++-- config.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/config.c b/config.c index 29f3b547acfc91..10646987916a80 100644 --- a/config.c +++ b/config.c @@ -1690,12 +1690,14 @@ static int do_git_config_sequence(const struct config_options *opts, current_parsing_scope = CONFIG_SCOPE_SYSTEM; if (git_config_system()) { + unsigned flag = opts->system_gently ? ACCESS_EACCES_OK : 0; + if (git_program_data_config() && - !access_or_die(git_program_data_config(), R_OK, 0)) + !access_or_die(git_program_data_config(), R_OK, flag)) ret += git_config_from_file(fn, git_program_data_config(), data); - if (!access_or_die(git_etc_gitconfig(), R_OK, 0)) + if (!access_or_die(git_etc_gitconfig(), R_OK, flag)) ret += git_config_from_file(fn, git_etc_gitconfig(), data); } @@ -1827,6 +1829,7 @@ void read_very_early_config(config_fn_t cb, void *data) opts.ignore_repo = 1; opts.ignore_worktree = 1; opts.ignore_cmdline = 1; + opts.system_gently = 1; config_with_options(cb, data, NULL, &opts); } diff --git a/config.h b/config.h index d706db8067e2b7..35fbbcb12787c7 100644 --- a/config.h +++ b/config.h @@ -58,6 +58,7 @@ struct config_options { unsigned int ignore_repo : 1; unsigned int ignore_worktree : 1; unsigned int ignore_cmdline : 1; + unsigned int system_gently : 1; const char *commondir; const char *git_dir; config_parser_event_fn_t event_fn;