From b710fe81677bb1c5c824e7db400ca0859635cd53 Mon Sep 17 00:00:00 2001 From: Adam Murray Date: Fri, 10 Jan 2025 07:28:20 +0000 Subject: [PATCH 1/3] trace2: prevent segfault on config collection where no value specified When TRACE2 analytics is enabled, a git config option that has no value causes a segfault. Steps to Reproduce GIT_TRACE2=true GIT_TRACE2_CONFIG_PARAMS=status.* git -c status.relativePaths version Expected Result git version 2.46.0 Actual Result zsh: segmentation fault GIT_TRACE2=true This adds checks to prevent the segfault and instead return an empty value. Signed-off-by: Adam Murray Signed-off-by: Johannes Schindelin --- t/t0210-trace2-normal.sh | 8 ++++++++ trace2.c | 2 +- trace2/tr2_tgt_event.c | 3 ++- trace2/tr2_tgt_normal.c | 5 +++-- trace2/tr2_tgt_perf.c | 5 +++-- 5 files changed, 17 insertions(+), 6 deletions(-) diff --git a/t/t0210-trace2-normal.sh b/t/t0210-trace2-normal.sh index eff9a59dbd0d5d..b95f0f8a9dc3b0 100755 --- a/t/t0210-trace2-normal.sh +++ b/t/t0210-trace2-normal.sh @@ -243,6 +243,14 @@ test_expect_success 'bug messages followed by BUG() are written to trace2' ' test_cmp expect actual ' +test_expect_success 'empty configuration values are handled' ' + test_when_finished "rm trace2.normal actual expect" && + echo >expect && + GIT_TRACE2="$(pwd)/trace2.normal" GIT_TRACE2_CONFIG_PARAMS=foo.empty \ + git -c foo.empty config foo.empty >actual && + test_cmp expect actual +' + sane_unset GIT_TRACE2_BRIEF # Now test without environment variables and get all Trace2 settings diff --git a/trace2.c b/trace2.c index 82d16e2783d986..a21b1e91dbca20 100644 --- a/trace2.c +++ b/trace2.c @@ -764,7 +764,7 @@ void trace2_def_param_fl(const char *file, int line, const char *param, if (!trace2_enabled) return; - redacted = redact_arg(value); + redacted = value ? redact_arg(value): NULL; for_each_wanted_builtin (j, tgt_j) if (tgt_j->pfn_param_fl) diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c index 69ee40449fa4a7..5a0381791f7eb4 100644 --- a/trace2/tr2_tgt_event.c +++ b/trace2/tr2_tgt_event.c @@ -493,7 +493,8 @@ static void fn_param_fl(const char *file, int line, const char *param, event_fmt_prepare(event_name, file, line, NULL, &jw); jw_object_string(&jw, "scope", scope_name); jw_object_string(&jw, "param", param); - jw_object_string(&jw, "value", value); + if (value) + jw_object_string(&jw, "value", value); jw_end(&jw); tr2_dst_write_line(&tr2dst_event, &jw.json); diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c index baef48aa6989ce..924736ab36093b 100644 --- a/trace2/tr2_tgt_normal.c +++ b/trace2/tr2_tgt_normal.c @@ -307,8 +307,9 @@ static void fn_param_fl(const char *file, int line, const char *param, enum config_scope scope = kvi->scope; const char *scope_name = config_scope_name(scope); - strbuf_addf(&buf_payload, "def_param scope:%s %s=%s", scope_name, param, - value); + strbuf_addf(&buf_payload, "def_param scope:%s %s", scope_name, param); + if (value) + strbuf_addf(&buf_payload, "=%s", value); normal_io_write_fl(file, line, &buf_payload); strbuf_release(&buf_payload); } diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c index 298ae27f9d7f24..4eb9289f950505 100644 --- a/trace2/tr2_tgt_perf.c +++ b/trace2/tr2_tgt_perf.c @@ -448,8 +448,9 @@ static void fn_param_fl(const char *file, int line, const char *param, struct strbuf scope_payload = STRBUF_INIT; enum config_scope scope = kvi->scope; const char *scope_name = config_scope_name(scope); - - strbuf_addf(&buf_payload, "%s:%s", param, value); + strbuf_addstr(&buf_payload, param); + if (value) + strbuf_addf(&buf_payload, ":%s", value); strbuf_addf(&scope_payload, "%s:%s", "scope", scope_name); perf_io_write_fl(file, line, event_name, NULL, NULL, NULL, From 5b3ed306b0bc69205647eaf0173cd1ef2e69ba23 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 13 Jan 2025 01:26:01 -0500 Subject: [PATCH 2/3] grep: prevent `^$` false match at end of file In some implementations, `regexec_buf()` assumes that it is fed lines; Without `REG_NOTEOL` it thinks the end of the buffer is the end of a line. Which makes sense, but trips up this case because we are not feeding lines, but rather a whole buffer. So the final newline is not the start of an empty line, but the true end of the buffer. This causes an interesting bug: $ echo content >file.txt $ git grep --no-index -n '^$' file.txt file.txt:2: This bug is fixed by making the end of the buffer consistently the end of the final line. The patch was applied from https://lore.kernel.org/git/20250113062601.GD767856@coredump.intra.peff.net/ Reported-by: Olly Betts Signed-off-by: Johannes Schindelin --- grep.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/grep.c b/grep.c index 4e155ee9e66367..c4bb9f10814024 100644 --- a/grep.c +++ b/grep.c @@ -1646,6 +1646,8 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle bol = gs->buf; left = gs->size; + if (left && gs->buf[left-1] == '\n') + left--; while (left) { const char *eol; int hit; From 4573f9d5898599bd08faf9588330dd81803685a8 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 21 Jan 2025 16:52:35 -0500 Subject: [PATCH 3/3] update-ref: do set reflog's `old_oid` In git 2.48.1, the `git update-ref` subcommand no longer correctly updates the reflog in some cases. Specifically, it appears that the `old_oid` field will not be updated when modifying a branch referenced by another symbolic ref (e.g. HEAD). This doesn't break the `git reflog` subcommand, but does break references like `HEAD@{1}`, which appear to read the `old_oid` field: git init -b main git commit --allow-empty -m "A" git commit --allow-empty -m "B" git update-ref -m "reason" refs/heads/main HEAD~ HEAD The `old_oid` field is now empty (all zeroes). This is only the case in derived reflogs (in this case .git/logs/HEAD). The reflog for `refs/heads/main` appears to be updated correctly. This was broken in 297c09eabb (refs: allow multiple reflog entries for the same refname, 2024-12-16). The reason for that was that there was assumed the flow of `lock_ref_for_update()` for reflog only updates was to capture the lock only. But this is wrong since this misses the `old_oid` population. As such this patch is the correct fix. Reported-by: Nika Layzell Acked-by: Karthik Nayak Signed-off-by: Johannes Schindelin --- refs/files-backend.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 5cfb8b7ca8678e..29f08dced40418 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2615,9 +2615,6 @@ static int lock_ref_for_update(struct files_ref_store *refs, update->backend_data = lock; - if (update->flags & REF_LOG_ONLY) - goto out; - if (update->type & REF_ISSYMREF) { if (update->flags & REF_NO_DEREF) { /*