From patchwork Mon Feb 3 21:17:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 11363485 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E825517E0 for ; Mon, 3 Feb 2020 21:18:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C779D2086A for ; Mon, 3 Feb 2020 21:18:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=ttaylorr-com.20150623.gappssmtp.com header.i=@ttaylorr-com.20150623.gappssmtp.com header.b="koR+fgbT" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727102AbgBCVR7 (ORCPT ); Mon, 3 Feb 2020 16:17:59 -0500 Received: from mail-pl1-f194.google.com ([209.85.214.194]:33790 "EHLO mail-pl1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726287AbgBCVR6 (ORCPT ); Mon, 3 Feb 2020 16:17:58 -0500 Received: by mail-pl1-f194.google.com with SMTP id ay11so6361506plb.0 for ; Mon, 03 Feb 2020 13:17:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=gMTQQpXQF+2KXtWvFP7ISqUU9kHh4GSMZaPwUsIlM/U=; b=koR+fgbTlKf3nKmu/+dk7YRQGBOVyyYBaVe72B2OoHBQZX9t+FwA+yBcSa0r4ZTkGi LZetb+q0jLNHsTsdJWaPTIFLIqAi5opb5DXslJHUhVnH84qPM6TXe1YYH6MNzUnRKNyV VdYdRKFqOiALK/530DMOvs6YyCHn9VmHIVHE6LxUGbBv90hlknfOCPQL6fE9OjomwgY+ 68bkVRTQcaxx1EWkk3ird7wITdn/Q5eZ42VEjcVEO+llTREBBHx/SAMKfJ/K1kVi5y1y 58y9eDFHeLFhdVKYk8ZoVmoOyOf8hF0jtb4iNX1oy6cheZNSIZHlirur0YU7S7hunyJ0 0DxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=gMTQQpXQF+2KXtWvFP7ISqUU9kHh4GSMZaPwUsIlM/U=; b=AlrmxGmEzN249+C8Sx8FxY6jFCU8MKejmBo34tts4uSm3Hvrlg2BL4cRhTOfixvJuU IMrrARHTbjOP/nRzQQ7LXdEJIrAKy1sLllnIGxoDBYmSh5H5vPc1ieqVdc4Q6mU9Dq9b TtE6It7xodgEssVmYObIekTFUt7Li8LX3/cg5VunQGS2KjuEtNPBCfoMVRhI8hjUXJ9u NDuZIjTmgSsFzQUeJgnXMEESOV+9UTtmvbYWqk/1g0crbpfvAiEIzOij+atL59nCe3m1 xRo34y6U53K+eGi9LOVH+Rrgoz/XrAoLpq3dT5AAysJnUhQ1n/rh5QIA+U30d5X/FJQ2 UMUQ== X-Gm-Message-State: APjAAAWS4td4RyxRkv3L+dHgByUBcVS8yyaiv1K+Xp0oRPZmsXkSooUP PrDWIVDGMWpjsPUMPHm9+5leL+z70Igfmg== X-Google-Smtp-Source: APXvYqxYtR2iU2Hn2u4w1/RQQhL4oZyDwiSL8Baz9t4USPgfIELtB7O4KSxqjtdaPIm4DYTx405jDg== X-Received: by 2002:a17:902:694c:: with SMTP id k12mr24817871plt.329.1580764677835; Mon, 03 Feb 2020 13:17:57 -0800 (PST) Received: from localhost ([205.175.106.126]) by smtp.gmail.com with ESMTPSA id s18sm999028pgn.34.2020.02.03.13.17.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Feb 2020 13:17:57 -0800 (PST) Date: Mon, 3 Feb 2020 13:17:56 -0800 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com, martin.agren@gmail.com Subject: [PATCH v2 1/5] t5318: don't pass non-object directory to '--object-dir' Message-ID: <84a8709ad14c8731e394d21b9ddee1372e0ec436.1580764494.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In f237c8b6fe (commit-graph: implement git-commit-graph write, 2018-04-02) the test t5318.3 was introduced to ensure that calling 'git commit-graph write' in a repository with no packfiles does not write any commit-graph file(s). To exercise more paths in 'builtin/commit-graph.c', this test passes '--object-dir' to 'git commit-graph write', but the given argument refers to the working copy, not the object directory. Since the commit-graph sub-commands currently swallow these errors, this does not result in a test failure. But, it is only lucky that the test ends with no commit-graphs, since there were none to begin with. In preparation for a future commit where an '--object-dir' argument that does not match a known object directory will print out a failure, let's fix the test to still use '--object-dir', but pass the correct location to the object store instead of '.'. Signed-off-by: Taylor Blau --- t/t5318-commit-graph.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index 3f03de6018..0bf98b56ec 100755 --- a/t/t5318-commit-graph.sh +++ b/t/t5318-commit-graph.sh @@ -19,8 +19,8 @@ test_expect_success 'verify graph with no graph file' ' test_expect_success 'write graph with no packs' ' cd "$TRASH_DIRECTORY/full" && - git commit-graph write --object-dir . && - test_path_is_missing info/commit-graph + git commit-graph write --object-dir $objdir && + test_path_is_missing $objdir/info/commit-graph ' test_expect_success 'exit with correct error on bad input to --stdin-packs' ' From patchwork Mon Feb 3 21:17:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 11363487 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1880314B4 for ; Mon, 3 Feb 2020 21:18:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E11082086A for ; Mon, 3 Feb 2020 21:18:02 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=ttaylorr-com.20150623.gappssmtp.com header.i=@ttaylorr-com.20150623.gappssmtp.com header.b="EHdr70fX" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727151AbgBCVSB (ORCPT ); Mon, 3 Feb 2020 16:18:01 -0500 Received: from mail-pf1-f194.google.com ([209.85.210.194]:35666 "EHLO mail-pf1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726834AbgBCVSB (ORCPT ); Mon, 3 Feb 2020 16:18:01 -0500 Received: by mail-pf1-f194.google.com with SMTP id y73so8226793pfg.2 for ; Mon, 03 Feb 2020 13:18:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=T+Xsn6rPsTA5jPBZt7kc7D6lqs/+LzNuNY7yXMe6AJs=; b=EHdr70fX4ODZd8eNuijWAQvgnTV+UYoGX7Cccw2nN25SXZBMGpVftnVKV8sRqszGby uUWA0PUDCWWjxsKFSPZ3M4/1joi9wjz9HqCKSfBPDOz1xVvXUQS1V3i3dE+unIsx6pI9 9xrsZy/xCXa46uuIQYJc4bJ3b0hhUNlPo3DhQDXkyEvFBq8uyzuWvOE8Pb1BRYRoqKzi iaTU+uWis4GjAPIyBUPd9rfQXsq2LTYlD4DZgFp8zt0bVlfZfl8CF/A7DszJxCPDzYdg MGKNcGhULBT2WP8y/U+TDhf44noqCmgbj5CxleEobhpTIwKERFKvnvP8Ll23NpIK7jP2 zadw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=T+Xsn6rPsTA5jPBZt7kc7D6lqs/+LzNuNY7yXMe6AJs=; b=fZPihaVkWIuWFPRs9cIj8saDaIEn4qMoL7WA3fZKcObqbmfUJhJqKXCiIQADLlwLjs cLhjWJAtlTwNxoPmWYrt/0fFlQD2n3GhUroBxSyyGJ0R+MTa41bFp/HiNFzTQHvP1VA6 F9bLAZk8Zaa1w9HLsKw7bx5lLYJvusHnn5REOrh0JTMZh/k9SKPAY1Jshwj+PKlbwsc/ JFxxQv9lkMZ66L/cggKPZFXYIIhMcNfxXXN11Z3Jaa/bRTWQV986YQGHQhrShcufiCk3 rPZLaeGq/JV8v9vITRypcJlM7gU3gPnZvoeFM7nemcYwLsoHWQszjuvBnC383pXIK0bT YUnA== X-Gm-Message-State: APjAAAWDeXzHMFxIy6XAoFeMQT+Nn2XDeo/xYOLQ8XOrkMXzVE09Ounx akDSk7NxF5EvTzU5pYS08YVK/28GexYTbg== X-Google-Smtp-Source: APXvYqy6lOcoD5K5+Rzyjtopmk+Y38WpYeSVRUfQcH+jjgeAapji/p/ns6JH0aJbQjaTO+aBDM8Xmg== X-Received: by 2002:aa7:9a8b:: with SMTP id w11mr24900124pfi.38.1580764680084; Mon, 03 Feb 2020 13:18:00 -0800 (PST) Received: from localhost ([205.175.106.126]) by smtp.gmail.com with ESMTPSA id 199sm17831759pfu.71.2020.02.03.13.17.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Feb 2020 13:17:59 -0800 (PST) Date: Mon, 3 Feb 2020 13:17:59 -0800 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com, martin.agren@gmail.com Subject: [PATCH v2 2/5] commit-graph.h: store an odb in 'struct write_commit_graph_context' Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org There are lots of places in 'commit-graph.h' where a function either has (or almost has) a full 'struct object_directory *', accesses '->path', and then throws away the rest of the struct. This can cause headaches when comparing the locations of object directories across alternates (e.g., in the case of deciding if two commit-graph layers can be merged). These paths are normalized with 'normalize_path_copy()' which mitigates some comparison issues, but not all [1]. Replace usage of 'char *object_dir' with 'odb->path' by storing a 'struct object_directory *' in the 'write_commit_graph_context' structure. This is an intermediate step towards getting rid of all path normalization in 'commit-graph.c'. Resolving a user-provided '--object-dir' argument now requires that we compare it to the known alternates for equality. Prior to this patch, an unknown '--object-dir' argument would silently exit with status zero. This can clearly lead to unintended behavior, such as verifying commit-graphs that aren't in a repository's own object store (or one of its alternates), or causing a typo to mask a legitimate commit-graph verification failure. Make this error non-silent by 'die()'-ing when the given '--object-dir' does not match any known alternate object store. [1]: In my testing, for example, I can get one side of the commit-graph code to fill object_dir with "./objects" and the other with just "objects". Signed-off-by: Taylor Blau --- Documentation/git-commit-graph.txt | 5 +++- builtin/commit-graph.c | 32 +++++++++++++++++++---- builtin/commit.c | 2 +- builtin/fetch.c | 2 +- builtin/gc.c | 2 +- commit-graph.c | 41 ++++++++++++------------------ commit-graph.h | 5 ++-- 7 files changed, 53 insertions(+), 36 deletions(-) diff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt index bcd85c1976..28d1fee505 100644 --- a/Documentation/git-commit-graph.txt +++ b/Documentation/git-commit-graph.txt @@ -26,7 +26,10 @@ OPTIONS file. This parameter exists to specify the location of an alternate that only has the objects directory, not a full `.git` directory. The commit-graph file is expected to be in the `/info` directory and - the packfiles are expected to be in `/pack`. + the packfiles are expected to be in `/pack`. If the directory + could not be made into an absolute path, or does not match any known + object directory, `git commit-graph ...` will exit with non-zero + status. --[no-]progress:: Turn progress on/off explicitly. If neither is specified, progress is diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index e0c6fc4bbf..31b57e4e1d 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -34,9 +34,28 @@ static struct opts_commit_graph { int progress; } opts; +struct object_directory *find_odb(struct repository *r, const char *obj_dir) +{ + struct object_directory *odb; + char *obj_dir_real = real_pathdup(obj_dir, 1); + + prepare_alt_odb(r); + for (odb = r->objects->odb; odb; odb = odb->next) { + if (!strcmp(obj_dir_real, real_path(odb->path))) + break; + } + + free(obj_dir_real); + + if (!odb) + die(_("could not find object directory matching %s"), obj_dir); + return odb; +} + static int graph_verify(int argc, const char **argv) { struct commit_graph *graph = NULL; + struct object_directory *odb = NULL; char *graph_name; int open_ok; int fd; @@ -67,7 +86,8 @@ static int graph_verify(int argc, const char **argv) if (opts.progress) flags |= COMMIT_GRAPH_WRITE_PROGRESS; - graph_name = get_commit_graph_filename(opts.obj_dir); + odb = find_odb(the_repository, opts.obj_dir); + graph_name = get_commit_graph_filename(odb->path); open_ok = open_commit_graph(graph_name, &fd, &st); if (!open_ok && errno != ENOENT) die_errno(_("Could not open commit-graph '%s'"), graph_name); @@ -76,8 +96,8 @@ static int graph_verify(int argc, const char **argv) if (open_ok) graph = load_commit_graph_one_fd_st(fd, &st); - else - graph = read_commit_graph_one(the_repository, opts.obj_dir); + else + graph = read_commit_graph_one(the_repository, odb->path); /* Return failure if open_ok predicted success */ if (!graph) @@ -94,6 +114,7 @@ static int graph_write(int argc, const char **argv) { struct string_list *pack_indexes = NULL; struct string_list *commit_hex = NULL; + struct object_directory *odb = NULL; struct string_list lines; int result = 0; enum commit_graph_write_flags flags = 0; @@ -145,9 +166,10 @@ static int graph_write(int argc, const char **argv) flags |= COMMIT_GRAPH_WRITE_PROGRESS; read_replace_refs = 0; + odb = find_odb(the_repository, opts.obj_dir); if (opts.reachable) { - if (write_commit_graph_reachable(opts.obj_dir, flags, &split_opts)) + if (write_commit_graph_reachable(odb, flags, &split_opts)) return 1; return 0; } @@ -169,7 +191,7 @@ static int graph_write(int argc, const char **argv) UNLEAK(buf); } - if (write_commit_graph(opts.obj_dir, + if (write_commit_graph(odb, pack_indexes, commit_hex, flags, diff --git a/builtin/commit.c b/builtin/commit.c index 646e84547d..9e124aaa7b 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1689,7 +1689,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) "not exceeded, and then \"git restore --staged :/\" to recover.")); if (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) && - write_commit_graph_reachable(get_object_directory(), 0, NULL)) + write_commit_graph_reachable(the_repository->objects->odb, 0, NULL)) return 1; repo_rerere(the_repository, 0); diff --git a/builtin/fetch.c b/builtin/fetch.c index b4c6d921d0..1ce16c96e9 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1870,7 +1870,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) if (progress) commit_graph_flags |= COMMIT_GRAPH_WRITE_PROGRESS; - write_commit_graph_reachable(get_object_directory(), + write_commit_graph_reachable(the_repository->objects->odb, commit_graph_flags, NULL); } diff --git a/builtin/gc.c b/builtin/gc.c index 3f76bf4aa7..8e0b9cf41b 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -686,7 +686,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) prepare_repo_settings(the_repository); if (the_repository->settings.gc_write_commit_graph == 1) - write_commit_graph_reachable(get_object_directory(), + write_commit_graph_reachable(the_repository->objects->odb, !quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0, NULL); diff --git a/commit-graph.c b/commit-graph.c index b205e65ed1..cbfeece112 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -772,7 +772,7 @@ struct packed_oid_list { struct write_commit_graph_context { struct repository *r; - char *obj_dir; + struct object_directory *odb; char *graph_name; struct packed_oid_list oids; struct packed_commit_list commits; @@ -1149,7 +1149,7 @@ static int add_ref_to_list(const char *refname, return 0; } -int write_commit_graph_reachable(const char *obj_dir, +int write_commit_graph_reachable(struct object_directory *odb, enum commit_graph_write_flags flags, const struct split_commit_graph_opts *split_opts) { @@ -1157,7 +1157,7 @@ int write_commit_graph_reachable(const char *obj_dir, int result; for_each_ref(add_ref_to_list, &list); - result = write_commit_graph(obj_dir, NULL, &list, + result = write_commit_graph(odb, NULL, &list, flags, split_opts); string_list_clear(&list, 0); @@ -1172,7 +1172,7 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx, struct strbuf packname = STRBUF_INIT; int dirlen; - strbuf_addf(&packname, "%s/pack/", ctx->obj_dir); + strbuf_addf(&packname, "%s/pack/", ctx->odb->path); dirlen = packname.len; if (ctx->report_progress) { strbuf_addf(&progress_title, @@ -1368,10 +1368,10 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) strbuf_addf(&tmp_file, "%s/info/commit-graphs/tmp_graph_XXXXXX", - ctx->obj_dir); + ctx->odb->path); ctx->graph_name = strbuf_detach(&tmp_file, NULL); } else { - ctx->graph_name = get_commit_graph_filename(ctx->obj_dir); + ctx->graph_name = get_commit_graph_filename(ctx->odb->path); } if (safe_create_leading_directories(ctx->graph_name)) { @@ -1382,7 +1382,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) } if (ctx->split) { - char *lock_name = get_chain_filename(ctx->obj_dir); + char *lock_name = get_chain_filename(ctx->odb->path); hold_lock_file_for_update(&lk, lock_name, LOCK_DIE_ON_ERROR); @@ -1506,12 +1506,12 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) } } } else { - char *graph_name = get_commit_graph_filename(ctx->obj_dir); + char *graph_name = get_commit_graph_filename(ctx->odb->path); unlink(graph_name); } ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1] = xstrdup(oid_to_hex(&file_hash)); - final_graph_name = get_split_graph_filename(ctx->obj_dir, + final_graph_name = get_split_graph_filename(ctx->odb->path, ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1]); ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 1] = final_graph_name; @@ -1553,7 +1553,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) while (g && (g->num_commits <= size_mult * num_commits || (max_commits && num_commits > max_commits))) { - if (strcmp(g->obj_dir, ctx->obj_dir)) + if (strcmp(g->obj_dir, ctx->odb->path)) break; num_commits += g->num_commits; @@ -1568,7 +1568,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) char *old_graph_name = get_commit_graph_filename(g->obj_dir); if (!strcmp(g->filename, old_graph_name) && - strcmp(g->obj_dir, ctx->obj_dir)) { + strcmp(g->obj_dir, ctx->odb->path)) { ctx->num_commit_graphs_after = 1; ctx->new_base_graph = NULL; } @@ -1719,13 +1719,13 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx) if (ctx->split_opts && ctx->split_opts->expire_time) expire_time -= ctx->split_opts->expire_time; if (!ctx->split) { - char *chain_file_name = get_chain_filename(ctx->obj_dir); + char *chain_file_name = get_chain_filename(ctx->odb->path); unlink(chain_file_name); free(chain_file_name); ctx->num_commit_graphs_after = 0; } - strbuf_addstr(&path, ctx->obj_dir); + strbuf_addstr(&path, ctx->odb->path); strbuf_addstr(&path, "/info/commit-graphs"); dir = opendir(path.buf); @@ -1764,7 +1764,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx) strbuf_release(&path); } -int write_commit_graph(const char *obj_dir, +int write_commit_graph(struct object_directory *odb, struct string_list *pack_indexes, struct string_list *commit_hex, enum commit_graph_write_flags flags, @@ -1772,7 +1772,6 @@ int write_commit_graph(const char *obj_dir, { struct write_commit_graph_context *ctx; uint32_t i, count_distinct = 0; - size_t len; int res = 0; if (!commit_graph_compatible(the_repository)) @@ -1780,14 +1779,7 @@ int write_commit_graph(const char *obj_dir, ctx = xcalloc(1, sizeof(struct write_commit_graph_context)); ctx->r = the_repository; - - /* normalize object dir with no trailing slash */ - ctx->obj_dir = xmallocz(strlen(obj_dir) + 1); - normalize_path_copy(ctx->obj_dir, obj_dir); - len = strlen(ctx->obj_dir); - if (len && ctx->obj_dir[len - 1] == '/') - ctx->obj_dir[len - 1] = 0; - + ctx->odb = odb; ctx->append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0; ctx->report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0; ctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0; @@ -1824,7 +1816,7 @@ int write_commit_graph(const char *obj_dir, ctx->oids.alloc = split_opts->max_commits; if (ctx->append) { - prepare_commit_graph_one(ctx->r, ctx->obj_dir); + prepare_commit_graph_one(ctx->r, ctx->odb->path); if (ctx->r->objects->commit_graph) ctx->oids.alloc += ctx->r->objects->commit_graph->num_commits; } @@ -1898,7 +1890,6 @@ int write_commit_graph(const char *obj_dir, free(ctx->graph_name); free(ctx->commits.list); free(ctx->oids.list); - free(ctx->obj_dir); if (ctx->commit_graph_filenames_after) { for (i = 0; i < ctx->num_commit_graphs_after; i++) { diff --git a/commit-graph.h b/commit-graph.h index 7f5c933fa2..2a6251bd3d 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -5,6 +5,7 @@ #include "repository.h" #include "string-list.h" #include "cache.h" +#include "object-store.h" #define GIT_TEST_COMMIT_GRAPH "GIT_TEST_COMMIT_GRAPH" #define GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD "GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD" @@ -91,10 +92,10 @@ struct split_commit_graph_opts { * is not compatible with the commit-graph feature, then the * methods will return 0 without writing a commit-graph. */ -int write_commit_graph_reachable(const char *obj_dir, +int write_commit_graph_reachable(struct object_directory *odb, enum commit_graph_write_flags flags, const struct split_commit_graph_opts *split_opts); -int write_commit_graph(const char *obj_dir, +int write_commit_graph(struct object_directory *odb, struct string_list *pack_indexes, struct string_list *commit_hex, enum commit_graph_write_flags flags, From patchwork Mon Feb 3 21:18:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 11363495 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A82E1921 for ; Mon, 3 Feb 2020 21:18:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7DF8C2087E for ; Mon, 3 Feb 2020 21:18:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=ttaylorr-com.20150623.gappssmtp.com header.i=@ttaylorr-com.20150623.gappssmtp.com header.b="lZvmp4Uv" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727084AbgBCVSF (ORCPT ); Mon, 3 Feb 2020 16:18:05 -0500 Received: from mail-pf1-f193.google.com ([209.85.210.193]:35668 "EHLO mail-pf1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726834AbgBCVSD (ORCPT ); Mon, 3 Feb 2020 16:18:03 -0500 Received: by mail-pf1-f193.google.com with SMTP id y73so8226856pfg.2 for ; Mon, 03 Feb 2020 13:18:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Rpt9qlFI/odbFse2IdV+tb0tpQ/vmzBN9ScpN0TY4RI=; b=lZvmp4UvsLtFE9mbfw02x/SxsFgsE9oZGzRNUanhlSOih2Ul9G8ZdtGOGHlQtQIdQq fnYwrivrva7Xo2PVhevCu064nwyk4KIqFdPYS2oNHcujc5cBzmpDn9/3ZhHWpue2RKH6 EzrSmLPrN624SV2R22u/wN3Dal8sf/Eks1tnIJpHeehAAotZUmKuMrAOgc1LXPVRH2FU AvmjmfsrMlZGUjvVaywzgSNVJz5vb93IRRMDDKTlw8YPrX8FMgy7Y2y+RyG2maMNuVbG 2ES2xHdfzfbwD2MLkvT61mxOMMGSPa9xRnGnzYdLfHm7rbEI3tRzwXe6QemfZQWsE/0c nVlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Rpt9qlFI/odbFse2IdV+tb0tpQ/vmzBN9ScpN0TY4RI=; b=BzQVOKpKYOxdo0YHhmsm6rZ50G8DPLm0XullJshKTjWHSbupXqH90PAIN1RRWrXGJ3 GbkTaZCcFQbRwnym0aHGq2JuxsuCznkWwsNlYYLbE/xmR9q06RF299/68mug4M3CEQbo /ivLS4N0SXVnj6pk/IPIckS7ymhE1B+RUcpj4ni7oBfIoZ1Va2Cjktm3pS3VtRbhvUPC y8eZwoVJLDwnW/62v5qSHZFVyd5cvVsJx3/kBq6wYqBdRzi2NQLAMjobGWls10ZPWQsD 4bTavio3QiaUlu9DIdjBjqAHUVSk2iKhVrl4+uzhqGNZiAk/a3RjKTSwlYLupFdw8ZoO EW6g== X-Gm-Message-State: APjAAAUchwFBIj8mQQgX0nMT0O+WJtP6WNo2ZOD0G1GQ3PleUItHk3no ckIzIBL1+hXTbx+kg4JRsL37A25Nb/JNEw== X-Google-Smtp-Source: APXvYqwlLeTVor7R+anbZcdjGho1WtHfwxqtlv+D0O+MkX9uwhJM+l/h0tWCqioOduM+UQoofh3oqw== X-Received: by 2002:a62:e30d:: with SMTP id g13mr26858960pfh.92.1580764681813; Mon, 03 Feb 2020 13:18:01 -0800 (PST) Received: from localhost ([205.175.106.126]) by smtp.gmail.com with ESMTPSA id y190sm21964577pfb.82.2020.02.03.13.18.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Feb 2020 13:18:01 -0800 (PST) Date: Mon, 3 Feb 2020 13:18:00 -0800 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com, martin.agren@gmail.com Subject: [PATCH v2 3/5] commit-graph.h: store object directory in 'struct commit_graph' Message-ID: <5fd5cfca6e9a59c0bbf62501e6064106cf87df49.1580764494.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In a previous patch, the 'char *object_dir' in 'struct commit_graph' was replaced with a 'struct object_directory'. This patch applies the same treatment to 'struct commit_graph', which is another intermediate step towards getting rid of all path normalization in 'commit-graph.c'. Instead of taking a 'char *object_dir', functions that construct a 'struct commit_graph' now take a 'struct object_directory *'. Any code that needs an object directory path use '->path' instead. This ensures that all calls to functions that perform path normalization are given arguments which do not themselves require normalization. This prepares those functions to drop their normalization entirely, which will occur in the subsequent patch. Signed-off-by: Taylor Blau --- builtin/commit-graph.c | 2 +- commit-graph.c | 38 +++++++++++++++++++++----------------- commit-graph.h | 5 +++-- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 31b57e4e1d..3501d9077b 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -97,7 +97,7 @@ static int graph_verify(int argc, const char **argv) if (open_ok) graph = load_commit_graph_one_fd_st(fd, &st); else - graph = read_commit_graph_one(the_repository, odb->path); + graph = read_commit_graph_one(the_repository, odb); /* Return failure if open_ok predicted success */ if (!graph) diff --git a/commit-graph.c b/commit-graph.c index cbfeece112..3af4a721ee 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -327,14 +327,15 @@ static struct commit_graph *load_commit_graph_one(const char *graph_file) return g; } -static struct commit_graph *load_commit_graph_v1(struct repository *r, const char *obj_dir) +static struct commit_graph *load_commit_graph_v1(struct repository *r, + struct object_directory *odb) { - char *graph_name = get_commit_graph_filename(obj_dir); + char *graph_name = get_commit_graph_filename(odb->path); struct commit_graph *g = load_commit_graph_one(graph_name); free(graph_name); if (g) - g->obj_dir = obj_dir; + g->odb = odb; return g; } @@ -372,14 +373,15 @@ static int add_graph_to_chain(struct commit_graph *g, return 1; } -static struct commit_graph *load_commit_graph_chain(struct repository *r, const char *obj_dir) +static struct commit_graph *load_commit_graph_chain(struct repository *r, + struct object_directory *odb) { struct commit_graph *graph_chain = NULL; struct strbuf line = STRBUF_INIT; struct stat st; struct object_id *oids; int i = 0, valid = 1, count; - char *chain_name = get_chain_filename(obj_dir); + char *chain_name = get_chain_filename(odb->path); FILE *fp; int stat_res; @@ -418,7 +420,7 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r, const free(graph_name); if (g) { - g->obj_dir = odb->path; + g->odb = odb; if (add_graph_to_chain(g, graph_chain, oids, i)) { graph_chain = g; @@ -442,23 +444,25 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r, const return graph_chain; } -struct commit_graph *read_commit_graph_one(struct repository *r, const char *obj_dir) +struct commit_graph *read_commit_graph_one(struct repository *r, + struct object_directory *odb) { - struct commit_graph *g = load_commit_graph_v1(r, obj_dir); + struct commit_graph *g = load_commit_graph_v1(r, odb); if (!g) - g = load_commit_graph_chain(r, obj_dir); + g = load_commit_graph_chain(r, odb); return g; } -static void prepare_commit_graph_one(struct repository *r, const char *obj_dir) +static void prepare_commit_graph_one(struct repository *r, + struct object_directory *odb) { if (r->objects->commit_graph) return; - r->objects->commit_graph = read_commit_graph_one(r, obj_dir); + r->objects->commit_graph = read_commit_graph_one(r, odb); } /* @@ -505,7 +509,7 @@ static int prepare_commit_graph(struct repository *r) for (odb = r->objects->odb; !r->objects->commit_graph && odb; odb = odb->next) - prepare_commit_graph_one(r, odb->path); + prepare_commit_graph_one(r, odb); return !!r->objects->commit_graph; } @@ -1470,7 +1474,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) if (ctx->split && ctx->base_graph_name && ctx->num_commit_graphs_after > 1) { char *new_base_hash = xstrdup(oid_to_hex(&ctx->new_base_graph->oid)); - char *new_base_name = get_split_graph_filename(ctx->new_base_graph->obj_dir, new_base_hash); + char *new_base_name = get_split_graph_filename(ctx->new_base_graph->odb->path, new_base_hash); free(ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 2]); free(ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 2]); @@ -1553,7 +1557,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) while (g && (g->num_commits <= size_mult * num_commits || (max_commits && num_commits > max_commits))) { - if (strcmp(g->obj_dir, ctx->odb->path)) + if (strcmp(g->odb->path, ctx->odb->path)) break; num_commits += g->num_commits; @@ -1565,10 +1569,10 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) ctx->new_base_graph = g; if (ctx->num_commit_graphs_after == 2) { - char *old_graph_name = get_commit_graph_filename(g->obj_dir); + char *old_graph_name = get_commit_graph_filename(g->odb->path); if (!strcmp(g->filename, old_graph_name) && - strcmp(g->obj_dir, ctx->odb->path)) { + strcmp(g->odb->path, ctx->odb->path)) { ctx->num_commit_graphs_after = 1; ctx->new_base_graph = NULL; } @@ -1816,7 +1820,7 @@ int write_commit_graph(struct object_directory *odb, ctx->oids.alloc = split_opts->max_commits; if (ctx->append) { - prepare_commit_graph_one(ctx->r, ctx->odb->path); + prepare_commit_graph_one(ctx->r, ctx->odb); if (ctx->r->objects->commit_graph) ctx->oids.alloc += ctx->r->objects->commit_graph->num_commits; } diff --git a/commit-graph.h b/commit-graph.h index 2a6251bd3d..2448134378 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -49,7 +49,7 @@ struct commit_graph { uint32_t num_commits; struct object_id oid; char *filename; - const char *obj_dir; + struct object_directory *odb; uint32_t num_commits_in_base; struct commit_graph *base_graph; @@ -62,7 +62,8 @@ struct commit_graph { }; struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st); -struct commit_graph *read_commit_graph_one(struct repository *r, const char *obj_dir); +struct commit_graph *read_commit_graph_one(struct repository *r, + struct object_directory *odb); struct commit_graph *parse_commit_graph(void *graph_map, int fd, size_t graph_size); From patchwork Mon Feb 3 21:18:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 11363491 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 703B6921 for ; Mon, 3 Feb 2020 21:18:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3BF992087E for ; Mon, 3 Feb 2020 21:18:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=ttaylorr-com.20150623.gappssmtp.com header.i=@ttaylorr-com.20150623.gappssmtp.com header.b="lcVUtDc6" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727215AbgBCVSH (ORCPT ); Mon, 3 Feb 2020 16:18:07 -0500 Received: from mail-pj1-f68.google.com ([209.85.216.68]:39089 "EHLO mail-pj1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725372AbgBCVSG (ORCPT ); Mon, 3 Feb 2020 16:18:06 -0500 Received: by mail-pj1-f68.google.com with SMTP id e9so320304pjr.4 for ; Mon, 03 Feb 2020 13:18:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=em6c8ZImZhOvxlvjBPGoFZifAkC+lBV7+zc6bNqtq18=; b=lcVUtDc6aLWycXD7s1RPy/EYb1xaFtN8ziCZaR40S4aOyLTORxB7WndUK5bCZ2TgeF 9hEby/7Gruqyzsn6nxNYdEVvUe5yzVvkInyi9Q+RT7PN5zEN6CjbOhLZWXqzbhgSYimg pq7+bYISNZJCdhF+xcQIwFJWYDsA4Or17npgWstSI0GLHFDRkWwMAqEvFAHr/AVPO+4w aiwVjOZmu+TgtPqxsp8JKJ6VMs+6jjxhdIMgMWl30iofwFasOVzJNx4n5reJjlvg6hwB UMggSbYeRXnnr8gxdaoeMlr5wFah1FIlss8PGAcTmQ8wHjC3HcaV54mip/voDzrP3bYe wVgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=em6c8ZImZhOvxlvjBPGoFZifAkC+lBV7+zc6bNqtq18=; b=Pv+rJyeCzsRdRsWNsg9HvA6QIsJ1ETNpjRPLE3T4U1uxPp/1dXAQEyPAZI7+xwXc/u Ad3lSdKqRRzJoV2WM1KbBsB3rbcvRdlzzGJuenBz7yXfW5ppEZvKnTrjHZpdk0HwbwNs ylecRAaJI2RsiVc6+uAm5nGl9kCTrj673+NpKZBEPYVJvErfaPWNl01uiDI7CSZBR/Sv xUWkbY12kairbuw2bUFCeQ8dIe3aeX6JDRdJ5laRKGZpuubksS8t5Tp4NmOa5rdhZ7Cp QBlDyIqIdlwiRnA4CHdc94Im7aLURn4BLw0hzz4BfeOdB/hs9AMCrpKn8XIfF46IuwuL iw3A== X-Gm-Message-State: APjAAAVwa0SUdxnAC1stNRujj5TRG76legQXREVB9oocwiX+qoLtbsa7 w12VF3VikRFPtvViNuiqo58Q83oCkhOwKQ== X-Google-Smtp-Source: APXvYqz/rsw96/r8xWIjHm7cplW1hQJ7DYshg2YgHRRgG8wo+diSbOlDrhifCi7yBJ24ryZnlRpHnw== X-Received: by 2002:a17:90a:d985:: with SMTP id d5mr1273189pjv.73.1580764683472; Mon, 03 Feb 2020 13:18:03 -0800 (PST) Received: from localhost ([205.175.106.126]) by smtp.gmail.com with ESMTPSA id b21sm22045422pfp.0.2020.02.03.13.18.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Feb 2020 13:18:03 -0800 (PST) Date: Mon, 3 Feb 2020 13:18:02 -0800 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com, martin.agren@gmail.com Subject: [PATCH v2 4/5] commit-graph.c: remove path normalization, comparison Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org As of the previous patch, all calls to 'commit-graph.c' functions which perform path normalization (for e.g., 'get_commit_graph_filename()') are of the form 'ctx->odb->path', which is always in normalized form. Now that there are no callers passing non-normalized paths to these functions, ensure that future callers are bound by the same restrictions by making these functions take a 'struct object_directory *' instead of a 'const char *'. To match, replace all calls with arguments of the form 'ctx->odb->path' with 'ctx->odb' To recover the path, functions that perform path manipulation simply use 'odb->path'. Further, avoid string comparisons with arguments of the form 'odb->path', and instead prefer raw pointer comparisons, which accomplish the same effect, but are far less brittle. This has a pleasant side-effect of making these functions much more robust to paths that cannot be normalized by 'normalize_path_copy()', i.e., because they are outside of the current working directory. For example, prior to this patch, Valgrind reports that the following uninitialized memory read [1]: $ ( cd t && GIT_DIR=../.git valgrind git rev-parse HEAD^ ) because 'normalize_path_copy()' can't normalize '../.git' (since it's relative to but above of the current working directory) [2]. By using a 'struct object_directory *' directly, 'get_commit_graph_filename()' does not need to normalize, because all paths are relative to the current working directory since they are always read from the '->path' of an object directory. [1]: https://lore.kernel.org/git/20191027042116.GA5801@sigill.intra.peff.net. [2]: The bug here is that 'get_commit_graph_filename()' returns the result of 'normalize_path_copy()' without checking the return value. Signed-off-by: Taylor Blau --- builtin/commit-graph.c | 2 +- commit-graph.c | 47 +++++++++++++++----------------------- commit-graph.h | 2 +- t/helper/test-read-graph.c | 6 ++--- 4 files changed, 24 insertions(+), 33 deletions(-) diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 3501d9077b..b16eba2a7a 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -87,7 +87,7 @@ static int graph_verify(int argc, const char **argv) flags |= COMMIT_GRAPH_WRITE_PROGRESS; odb = find_odb(the_repository, opts.obj_dir); - graph_name = get_commit_graph_filename(odb->path); + graph_name = get_commit_graph_filename(odb); open_ok = open_commit_graph(graph_name, &fd, &st); if (!open_ok && errno != ENOENT) die_errno(_("Could not open commit-graph '%s'"), graph_name); diff --git a/commit-graph.c b/commit-graph.c index 3af4a721ee..49541760b5 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -44,30 +44,21 @@ /* Remember to update object flag allocation in object.h */ #define REACHABLE (1u<<15) -char *get_commit_graph_filename(const char *obj_dir) +char *get_commit_graph_filename(struct object_directory *odb) { - char *filename = xstrfmt("%s/info/commit-graph", obj_dir); - char *normalized = xmalloc(strlen(filename) + 1); - normalize_path_copy(normalized, filename); - free(filename); - return normalized; + return xstrfmt("%s/info/commit-graph", odb->path); } -static char *get_split_graph_filename(const char *obj_dir, +static char *get_split_graph_filename(struct object_directory *odb, const char *oid_hex) { - char *filename = xstrfmt("%s/info/commit-graphs/graph-%s.graph", - obj_dir, - oid_hex); - char *normalized = xmalloc(strlen(filename) + 1); - normalize_path_copy(normalized, filename); - free(filename); - return normalized; + return xstrfmt("%s/info/commit-graphs/graph-%s.graph", odb->path, + oid_hex); } -static char *get_chain_filename(const char *obj_dir) +static char *get_chain_filename(struct object_directory *odb) { - return xstrfmt("%s/info/commit-graphs/commit-graph-chain", obj_dir); + return xstrfmt("%s/info/commit-graphs/commit-graph-chain", odb->path); } static uint8_t oid_version(void) @@ -330,7 +321,7 @@ static struct commit_graph *load_commit_graph_one(const char *graph_file) static struct commit_graph *load_commit_graph_v1(struct repository *r, struct object_directory *odb) { - char *graph_name = get_commit_graph_filename(odb->path); + char *graph_name = get_commit_graph_filename(odb); struct commit_graph *g = load_commit_graph_one(graph_name); free(graph_name); @@ -381,7 +372,7 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r, struct stat st; struct object_id *oids; int i = 0, valid = 1, count; - char *chain_name = get_chain_filename(odb->path); + char *chain_name = get_chain_filename(odb); FILE *fp; int stat_res; @@ -414,7 +405,7 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r, valid = 0; for (odb = r->objects->odb; odb; odb = odb->next) { - char *graph_name = get_split_graph_filename(odb->path, line.buf); + char *graph_name = get_split_graph_filename(odb, line.buf); struct commit_graph *g = load_commit_graph_one(graph_name); free(graph_name); @@ -1375,7 +1366,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) ctx->odb->path); ctx->graph_name = strbuf_detach(&tmp_file, NULL); } else { - ctx->graph_name = get_commit_graph_filename(ctx->odb->path); + ctx->graph_name = get_commit_graph_filename(ctx->odb); } if (safe_create_leading_directories(ctx->graph_name)) { @@ -1386,7 +1377,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) } if (ctx->split) { - char *lock_name = get_chain_filename(ctx->odb->path); + char *lock_name = get_chain_filename(ctx->odb); hold_lock_file_for_update(&lk, lock_name, LOCK_DIE_ON_ERROR); @@ -1474,7 +1465,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) if (ctx->split && ctx->base_graph_name && ctx->num_commit_graphs_after > 1) { char *new_base_hash = xstrdup(oid_to_hex(&ctx->new_base_graph->oid)); - char *new_base_name = get_split_graph_filename(ctx->new_base_graph->odb->path, new_base_hash); + char *new_base_name = get_split_graph_filename(ctx->new_base_graph->odb, new_base_hash); free(ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 2]); free(ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 2]); @@ -1510,12 +1501,12 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) } } } else { - char *graph_name = get_commit_graph_filename(ctx->odb->path); + char *graph_name = get_commit_graph_filename(ctx->odb); unlink(graph_name); } ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1] = xstrdup(oid_to_hex(&file_hash)); - final_graph_name = get_split_graph_filename(ctx->odb->path, + final_graph_name = get_split_graph_filename(ctx->odb, ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1]); ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 1] = final_graph_name; @@ -1557,7 +1548,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) while (g && (g->num_commits <= size_mult * num_commits || (max_commits && num_commits > max_commits))) { - if (strcmp(g->odb->path, ctx->odb->path)) + if (g->odb != ctx->odb) break; num_commits += g->num_commits; @@ -1569,10 +1560,10 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) ctx->new_base_graph = g; if (ctx->num_commit_graphs_after == 2) { - char *old_graph_name = get_commit_graph_filename(g->odb->path); + char *old_graph_name = get_commit_graph_filename(g->odb); if (!strcmp(g->filename, old_graph_name) && - strcmp(g->odb->path, ctx->odb->path)) { + g->odb != ctx->odb) { ctx->num_commit_graphs_after = 1; ctx->new_base_graph = NULL; } @@ -1723,7 +1714,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx) if (ctx->split_opts && ctx->split_opts->expire_time) expire_time -= ctx->split_opts->expire_time; if (!ctx->split) { - char *chain_file_name = get_chain_filename(ctx->odb->path); + char *chain_file_name = get_chain_filename(ctx->odb); unlink(chain_file_name); free(chain_file_name); ctx->num_commit_graphs_after = 0; diff --git a/commit-graph.h b/commit-graph.h index 2448134378..5a690723b0 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -12,7 +12,7 @@ struct commit; -char *get_commit_graph_filename(const char *obj_dir); +char *get_commit_graph_filename(struct object_directory *odb); int open_commit_graph(const char *graph_file, int *fd, struct stat *st); /* diff --git a/t/helper/test-read-graph.c b/t/helper/test-read-graph.c index d2884efe0a..2c2f65f06c 100644 --- a/t/helper/test-read-graph.c +++ b/t/helper/test-read-graph.c @@ -11,12 +11,12 @@ int cmd__read_graph(int argc, const char **argv) int open_ok; int fd; struct stat st; - const char *object_dir; + struct object_directory *odb; setup_git_directory(); - object_dir = get_object_directory(); + odb = the_repository->objects->odb; - graph_name = get_commit_graph_filename(object_dir); + graph_name = get_commit_graph_filename(odb); open_ok = open_commit_graph(graph_name, &fd, &st); if (!open_ok) From patchwork Mon Feb 3 21:18:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 11363493 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9F26517E0 for ; Mon, 3 Feb 2020 21:18:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7CC1F2084E for ; Mon, 3 Feb 2020 21:18:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=ttaylorr-com.20150623.gappssmtp.com header.i=@ttaylorr-com.20150623.gappssmtp.com header.b="dD89+Zsj" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727219AbgBCVSH (ORCPT ); Mon, 3 Feb 2020 16:18:07 -0500 Received: from mail-pl1-f196.google.com ([209.85.214.196]:41706 "EHLO mail-pl1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727207AbgBCVSG (ORCPT ); Mon, 3 Feb 2020 16:18:06 -0500 Received: by mail-pl1-f196.google.com with SMTP id t14so6344091plr.8 for ; Mon, 03 Feb 2020 13:18:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Rcue2sRtSBCn2qWsekG8m907Ng8gf/+kVKluGsHflS0=; b=dD89+Zsj3r5py+X1QQUIhx/PzbkweE6UVWVOucygbtrDu8EDOHLiFmQUKTWPm9zm+7 YAo71D5LwmYM6t7jKohceFMLT0jpmPwM4i0TPNQYf2v7MrqhTyqfa9/EsBoPWcomPhAJ +MgQlne1go0vhOwCcPxPxpC956iD4iEhbRR5wPM2LhCk9vCBQvJeO+I+Ceb+ng8TK4+c UwP1rTEJMxFUDRuX6Nm7IRF1A9Z+Op0VScwhKH7PnrNfiuDZZz4R1khLpXS5ptUHj5JR s1oksb6Wg0PSJapDvYdMGHEaV9ehaw10f+AcQTYepeV5DfVzGcsq6On5JGSPMfAVqVoU KzEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Rcue2sRtSBCn2qWsekG8m907Ng8gf/+kVKluGsHflS0=; b=Ox4HX5re7j2F8BT1r5SCxyZuwx084vD/bu7L+lPp5+DdTH9V/6hsjplDy5PbAeLQCW P27jY4m+O5SGhPNXE2T5rt/ydBjvGoEl8Vlb4GKOJaMOypV3E2zo8xbqd90Jb2FkrXkw muSHAKt19f/G1+9Zz8qH+fXR4utLi2pgwMjQItED6ujtZblhfnz90YolUfXP1PcYYdi3 9AgKz+9rJDRPYv3rEBwNXCfmH+9Mdp+JqQzU8KW5slTnqNhzBVbTGvYJaGY7N5+qe/Gq PxyRJC6ENvD8U+KdYZTAVbC/ZjyYBSxlxtrRfxlFwgSawB0Q9jlD/xbrIl+M+T6Neqrd q7kg== X-Gm-Message-State: APjAAAXfqWReBp8e4U58VR7bNPKK8JoHkXp3V68scQuP+gQoQbYQaXvk 3jG6YZJ58YeSFG7Uc0rrfnCmJBkevmPGgg== X-Google-Smtp-Source: APXvYqy1HvZc+6HVMuXh+1K6PGw6KYJ1LrUh/crN3YSihqWE8gbWF7/caAhMflUJbwcrJQT0qzg1zw== X-Received: by 2002:a17:902:34a:: with SMTP id 68mr24444535pld.250.1580764685077; Mon, 03 Feb 2020 13:18:05 -0800 (PST) Received: from localhost ([205.175.106.126]) by smtp.gmail.com with ESMTPSA id d2sm386341pjv.18.2020.02.03.13.18.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Feb 2020 13:18:04 -0800 (PST) Date: Mon, 3 Feb 2020 13:18:04 -0800 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com, martin.agren@gmail.com Subject: [PATCH v2 5/5] commit-graph.h: use odb in 'load_commit_graph_one_fd_st' Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Apply a similar treatment as in the previous patch to pass a 'struct object_directory *' through the 'load_commit_graph_one_fd_st' initializer, too. This prevents a potential bug where a pointer comparison is made to a NULL 'g->odb', which would cause the commit-graph machinery to think that a pair of commit-graphs belonged to different alternates when in fact they do not (i.e., in the case of no '--object-dir'). Signed-off-by: Taylor Blau --- builtin/commit-graph.c | 2 +- commit-graph.c | 21 ++++++++++----------- commit-graph.h | 3 ++- t/helper/test-read-graph.c | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index b16eba2a7a..dd6ab84be8 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -95,7 +95,7 @@ static int graph_verify(int argc, const char **argv) FREE_AND_NULL(graph_name); if (open_ok) - graph = load_commit_graph_one_fd_st(fd, &st); + graph = load_commit_graph_one_fd_st(fd, &st, odb); else graph = read_commit_graph_one(the_repository, odb); diff --git a/commit-graph.c b/commit-graph.c index 49541760b5..656dd647d5 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -108,7 +108,8 @@ int open_commit_graph(const char *graph_file, int *fd, struct stat *st) return 1; } -struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st) +struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st, + struct object_directory *odb) { void *graph_map; size_t graph_size; @@ -124,7 +125,9 @@ struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st) graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0); ret = parse_commit_graph(graph_map, fd, graph_size); - if (!ret) { + if (ret) + ret->odb = odb; + else { munmap(graph_map, graph_size); close(fd); } @@ -299,7 +302,8 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd, return graph; } -static struct commit_graph *load_commit_graph_one(const char *graph_file) +static struct commit_graph *load_commit_graph_one(const char *graph_file, + struct object_directory *odb) { struct stat st; @@ -310,7 +314,7 @@ static struct commit_graph *load_commit_graph_one(const char *graph_file) if (!open_ok) return NULL; - g = load_commit_graph_one_fd_st(fd, &st); + g = load_commit_graph_one_fd_st(fd, &st, odb); if (g) g->filename = xstrdup(graph_file); @@ -322,12 +326,9 @@ static struct commit_graph *load_commit_graph_v1(struct repository *r, struct object_directory *odb) { char *graph_name = get_commit_graph_filename(odb); - struct commit_graph *g = load_commit_graph_one(graph_name); + struct commit_graph *g = load_commit_graph_one(graph_name, odb); free(graph_name); - if (g) - g->odb = odb; - return g; } @@ -406,13 +407,11 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r, valid = 0; for (odb = r->objects->odb; odb; odb = odb->next) { char *graph_name = get_split_graph_filename(odb, line.buf); - struct commit_graph *g = load_commit_graph_one(graph_name); + struct commit_graph *g = load_commit_graph_one(graph_name, odb); free(graph_name); if (g) { - g->odb = odb; - if (add_graph_to_chain(g, graph_chain, oids, i)) { graph_chain = g; valid = 1; diff --git a/commit-graph.h b/commit-graph.h index 5a690723b0..e87a6f6360 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -61,7 +61,8 @@ struct commit_graph { const unsigned char *chunk_base_graphs; }; -struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st); +struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st, + struct object_directory *odb); struct commit_graph *read_commit_graph_one(struct repository *r, struct object_directory *odb); struct commit_graph *parse_commit_graph(void *graph_map, int fd, diff --git a/t/helper/test-read-graph.c b/t/helper/test-read-graph.c index 2c2f65f06c..f8a461767c 100644 --- a/t/helper/test-read-graph.c +++ b/t/helper/test-read-graph.c @@ -22,7 +22,7 @@ int cmd__read_graph(int argc, const char **argv) if (!open_ok) die_errno(_("Could not open commit-graph '%s'"), graph_name); - graph = load_commit_graph_one_fd_st(fd, &st); + graph = load_commit_graph_one_fd_st(fd, &st, odb); if (!graph) return 1;