From patchwork Thu Jan 30 23:00:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 11358875 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 82B40921 for ; Thu, 30 Jan 2020 23:00:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 610612063A for ; Thu, 30 Jan 2020 23:00:47 +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="BGgal6Kh" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727653AbgA3XAq (ORCPT ); Thu, 30 Jan 2020 18:00:46 -0500 Received: from mail-pl1-f195.google.com ([209.85.214.195]:39027 "EHLO mail-pl1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727566AbgA3XAq (ORCPT ); Thu, 30 Jan 2020 18:00:46 -0500 Received: by mail-pl1-f195.google.com with SMTP id g6so1922746plp.6 for ; Thu, 30 Jan 2020 15:00:46 -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=38zLZ7aCxl5GX/ZK9TIOtwDt7VD9dWxDyz/bNM5GiRo=; b=BGgal6Kh7j0Bp8+qpzUs7Yxjx/HnvMWV+rMgCQX86s+Oz0Pg2DD46BnTPf3raSkTAw Nu6aSfij9N9+Po4g2EHhAe5wwTeU/DG2QiwpkRVpNhiPq8MMw+xCfnWU72h2Qc9fYR6E EDyQ00PdFY+3MC9uQhbijAsAAq3VjZKHgZ9CHMT6Eq0huxHSn4OrAulcC5m9WIOoLLw/ oSvw9QSb7Oe0KoU2koiePaS5cQerEN+JC0XDb6vKdBKevdnhC7MDBPeEBnMzQgEaucbP asN1zBogSQICDpzk2fgTgAPxq+1Lo8nuU7TOUE86U28IdacW4Lmexk/TThTT6m7dI/05 8p1g== 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=38zLZ7aCxl5GX/ZK9TIOtwDt7VD9dWxDyz/bNM5GiRo=; b=hkxAPwLictuRYF6bo2xhLEyzoo8KNKU5GgREdWXhVwTPM4jfm912/aOhwFKdcmxB2b vYVbmFPmioryWvYkQpc//fnmL8vTXFptVilL9GDhSFuWg5fuH4fo4v5ScJBwUQ0jJw2S cs3mK/3q+us6M0iYqsu3vAtQJlAkP1TOhFO9tMQlPx2gSdGErH4SDPcM7mZ6RG9kdfIF e2vo0gs3IhcDhvy94OYGuroiQZSEZnu+04ZW1hj9MH50fYI29crOyEhpxy4Qkp/LDJFd lFYjKw3f/KOB6X31wosCcQDyUHcoZ7NAvgc9V/JWMKae+tOWm6sxtf9uUCkcnBdvpm7N jbzA== X-Gm-Message-State: APjAAAWpSdH/mHbkBrkdFcv4aZuGGaGEl55abVVP92Gx1QRf8ISj7sjX dhPtZJSbauxqyOReDgWCwjJE3pnnQ0QQDw== X-Google-Smtp-Source: APXvYqx7rKyk53leZ74Qqm9TeMORA1gP6DOFBfRgeg0cXAPy0a4cJ9N28mf9PAdn2O6BI5LWBDy79g== X-Received: by 2002:a17:90a:b009:: with SMTP id x9mr8669320pjq.124.1580425245051; Thu, 30 Jan 2020 15:00:45 -0800 (PST) Received: from localhost ([2601:602:9200:32b0:5c8f:7dac:47b8:95ff]) by smtp.gmail.com with ESMTPSA id y2sm7847992pff.139.2020.01.30.15.00.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Jan 2020 15:00:44 -0800 (PST) Date: Thu, 30 Jan 2020 15:00:43 -0800 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com Subject: [PATCH 1/6] t5318: don't pass non-object directory to '--object-dir' Message-ID: <7b22efb0c61046a5864afeb052332a3a53331533.1580424766.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 Thu Jan 30 23:00:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 11358881 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 6AA4B924 for ; Thu, 30 Jan 2020 23:00:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3E9B720CC7 for ; Thu, 30 Jan 2020 23:00:54 +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="qFkfxtN9" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727722AbgA3XAx (ORCPT ); Thu, 30 Jan 2020 18:00:53 -0500 Received: from mail-pj1-f66.google.com ([209.85.216.66]:52664 "EHLO mail-pj1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727525AbgA3XAx (ORCPT ); Thu, 30 Jan 2020 18:00:53 -0500 Received: by mail-pj1-f66.google.com with SMTP id ep11so1971962pjb.2 for ; Thu, 30 Jan 2020 15:00:52 -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=cW4oxAZ+Ys9mN8o11f40V2w4JU4mKeA8zVfZFWdQVxM=; b=qFkfxtN9LnMdbMQP+VbDRrWILDvoC/hhBqakLPZNqioZd5SuWo1cLEh/4AthO64KxZ 7dECWMCwJ85NFjNmhsflh6XAh29hD5VM0zEuVuYLzjr0ll03QLlzV24oAhEIi6zPbvaS 8ED0ccT3PyEWn43Rs/JSgm9NmiBmNdM3vZ551FCcOCN2bpSjGnpyuoyGEta/8ZmS4wks NkwdOYDP1jrMIth+bD8NGfiCXQ6uWG0TC2Tg9KoJhXOLGiDZfI7F129zuEBoM6om354F 818dV69vwqW7sUsOImDB5DuX8dkOBK1wkz1y+BDs1Mx1m5JrV2SmXQRfFVtdWsW5B93s BbKA== 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=cW4oxAZ+Ys9mN8o11f40V2w4JU4mKeA8zVfZFWdQVxM=; b=idAoZv1dNBFDsC2uyotgLLnYfMbm1apUo0W1chOjIT3qKpbxfwAkKcXXtW+hhW0nQl 3WShHpHpS1SIjVHK+40agWGtV3rUJBRuOlJQrqTts8Pc5nHsJHMchzeBVEFxYTXhorI/ 0zNkI0nfXGQPB7x83Ii6kuhcyRkl0SHkBx/DUWuljA+/nIisKPPLARz8BPWx3frsMYyT lXSv3nrrLAirLHSt/yUjGU2pVqnqG3MC58/iBa4SrGndgF+wZedFlG0cV7MIoDfz52Ya qoKwJTHc6GOnE4xV1sIstxGu+KgH2lAfcdD0kj8Wv+hmHOath0Dv+LmF0F3EZGv8a3yq 6/qA== X-Gm-Message-State: APjAAAUM0UWTKp5MD5AIZU+Pv8eT9+IxheK/BrFZ+eoOpIxhTi/WVf9H Bp9MxzgyzNvr3ZqlyMKdjHg9PK6PQNpGNg== X-Google-Smtp-Source: APXvYqx4YkKsSKdEX3GzT98pLoGoQ9S3mTxe1pYqHVEc1UkKYPyR+NhAkEVGgUTaL1+CGsJWzd67hw== X-Received: by 2002:a17:90a:db49:: with SMTP id u9mr8300247pjx.13.1580425251684; Thu, 30 Jan 2020 15:00:51 -0800 (PST) Received: from localhost ([2601:602:9200:32b0:5c8f:7dac:47b8:95ff]) by smtp.gmail.com with ESMTPSA id 133sm7790090pfy.14.2020.01.30.15.00.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Jan 2020 15:00:51 -0800 (PST) Date: Thu, 30 Jan 2020 15:00:50 -0800 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com Subject: [PATCH 2/6] commit-graph.h: store object directory in 'struct commit_graph' Message-ID: <890e0e7136204f5ca47f0703f32b4adb99ad8d7e.1580424766.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 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]. Instead of getting rid of the 'struct object_directory *', store that insead of a 'char *odb' in 'struct commit_graph'. Once the 'struct write_commit_graph_context' has an object_directory pointer, too, this will allow calling code to replace these error-prone path comparisons with raw pointer comparisons, thereby circumventing any normalization-related errors. This will be introduced in a subsequent patch. [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 --- builtin/commit-graph.c | 13 +++++++--- builtin/commit.c | 1 + commit-graph.c | 59 ++++++++++++++++++++++++++++++------------ commit-graph.h | 8 ++++-- 4 files changed, 58 insertions(+), 23 deletions(-) diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index e0c6fc4bbf..3edac318e8 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -76,8 +76,11 @@ 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 { + struct object_directory *odb; + if ((odb = find_odb(the_repository, opts.obj_dir))) + graph = read_commit_graph_one(the_repository, odb); + } /* Return failure if open_ok predicted success */ if (!graph) @@ -97,6 +100,7 @@ static int graph_write(int argc, const char **argv) struct string_list lines; int result = 0; enum commit_graph_write_flags flags = 0; + struct object_directory *odb = NULL; static struct option builtin_commit_graph_write_options[] = { OPT_STRING(0, "object-dir", &opts.obj_dir, @@ -145,9 +149,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->path, flags, &split_opts)) return 1; return 0; } @@ -169,7 +174,7 @@ static int graph_write(int argc, const char **argv) UNLEAK(buf); } - if (write_commit_graph(opts.obj_dir, + if (write_commit_graph(odb->path, pack_indexes, commit_hex, flags, diff --git a/builtin/commit.c b/builtin/commit.c index aa1332308a..bd071169d7 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -36,6 +36,7 @@ #include "help.h" #include "commit-reach.h" #include "commit-graph.h" +#include "object-store.h" static const char * const builtin_commit_usage[] = { N_("git commit [] [--] ..."), diff --git a/commit-graph.c b/commit-graph.c index b205e65ed1..2c06876b26 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -75,6 +75,26 @@ static uint8_t oid_version(void) return 1; } +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); + int cmp = -1; + + prepare_alt_odb(r); + for (odb = r->objects->odb; odb; odb = odb->next) { + cmp = strcmp(obj_dir_real, real_path(odb->path)); + if (!cmp) + break; + } + + free(obj_dir_real); + + if (cmp) + odb = NULL; + return odb; +} + static struct commit_graph *alloc_commit_graph(void) { struct commit_graph *g = xcalloc(1, sizeof(*g)); @@ -327,14 +347,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 +393,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 +440,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 +464,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 +529,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 +1494,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 +1577,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->odb->path, ctx->obj_dir)) break; num_commits += g->num_commits; @@ -1565,10 +1589,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->obj_dir)) { + strcmp(g->odb->path, ctx->obj_dir)) { ctx->num_commit_graphs_after = 1; ctx->new_base_graph = NULL; } @@ -1824,7 +1848,8 @@ 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); + struct object_directory *odb = find_odb(ctx->r, ctx->obj_dir); + prepare_commit_graph_one(ctx->r, 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 7f5c933fa2..9700a6c7c2 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" @@ -14,6 +15,8 @@ struct commit; char *get_commit_graph_filename(const char *obj_dir); int open_commit_graph(const char *graph_file, int *fd, struct stat *st); +struct object_directory *find_odb(struct repository *r, const char *obj_dir); + /* * Given a commit struct, try to fill the commit struct info, including: * 1. tree object @@ -48,7 +51,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; @@ -61,7 +64,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 Thu Jan 30 23:00:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 11358885 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 3DA65921 for ; Thu, 30 Jan 2020 23:00:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1CAE220CC7 for ; Thu, 30 Jan 2020 23:00:58 +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="aHYHLHU8" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727814AbgA3XA5 (ORCPT ); Thu, 30 Jan 2020 18:00:57 -0500 Received: from mail-pf1-f193.google.com ([209.85.210.193]:38864 "EHLO mail-pf1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727781AbgA3XA4 (ORCPT ); Thu, 30 Jan 2020 18:00:56 -0500 Received: by mail-pf1-f193.google.com with SMTP id x185so2260724pfc.5 for ; Thu, 30 Jan 2020 15:00:56 -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=iCiY0lDE2y7kIK/mcEsUBELJibebaw3Wond40rF8buk=; b=aHYHLHU8ajM3QoqDs/t8xH6x6UsriokwM0QRtm8tpyhdyda51tU+0IUYz7PJmS09Hi JsNGc5yoUJchnkwUaVzO/4av3qc/nv/cc9juvNnFeTQWAM3sD/I2c0dQ6Fdp8A0rWWjB 6lXE1D7YLGpNY5wtB927/oHf5wCR3oXkVoRRqRQ32uqJF+g+hVMhYo2QLNfm22Xg14OF Lr+nBEIpm7fycw3WPdoJB0Yzd9UxyYJkwfxiXzx/J8Thrwwe8/SZAwzwQjow7nF7VQih iHlUbsXPx4XWNGj+O322vM6mqNGYKPKhMbbtMH3Zqd+pb9yGDbHG7gDKkTjEccCLu9cZ u/+Q== 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=iCiY0lDE2y7kIK/mcEsUBELJibebaw3Wond40rF8buk=; b=pGiEDaPMEbrsSTXeYyZR38Xd0P9d8c1qbVj8SMYDycYdgU8bcsmL3UdO8kRDMP/FTy e1Qp3//ISxh1e3azXjst0SbJx02OE+x9RAinR1BcEFzcpk+FiSRLapSITeGIWTQpo7sL zEHPxkgoJRL3TNvn8w85bbbKIHn+v0ItV6uDE/qbd+id/hWJxPtQTSE0m+dVCgJZvZSO LC5nhLkUp22JKO4uqI67DmitiNo9jMWWW0dHwBm6sWV14QDGRBj/+nLl7/VI9eZ7TtUj KDkdVpaXZcC5FiE7Cy1yXdcDc53i3Jk8wYsuDLN7/TYQoztVUP4Q1fP0el5gReiJv5tV T/Rw== X-Gm-Message-State: APjAAAX1Xl7q7FsiZkCegH4wera+ZugGLNkkN4gpzhPkE8TOX+iJneoD Y4MvRRwft8kubxcElWbJrSIiRgLTMsAHcw== X-Google-Smtp-Source: APXvYqzwKU70HzjZABck/rkzv+tzYkm/ZhwgHyyVkaYTGJXnRah47UX0OBn+gQpAr4D7gjXhSCm7SQ== X-Received: by 2002:a63:1f0c:: with SMTP id f12mr7276657pgf.247.1580425255888; Thu, 30 Jan 2020 15:00:55 -0800 (PST) Received: from localhost ([2601:602:9200:32b0:5c8f:7dac:47b8:95ff]) by smtp.gmail.com with ESMTPSA id v25sm7554420pfe.147.2020.01.30.15.00.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Jan 2020 15:00:55 -0800 (PST) Date: Thu, 30 Jan 2020 15:00:54 -0800 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com Subject: [PATCH 3/6] builtin/commit-graph.c: die() with unknown '--object-dir' 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 The commit-graph sub-commands 'write', 'verify' both take an '--object-dir' argument, which is used to specify the location of an object directory containing commit-graphs. However, there was no verification that the '--object-dir' argument was an object directory. In the case of an '--object-dir' argument that either (1) doesn't exist, or (2) isn't an object directory, 'git commit-graph ...' would exit silently 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. To remedy this, let's wrap 'find_odb()' with 'find_odb_or_die()' and cause the above such errors to produce an error and non-zero exit code. Signed-off-by: Taylor Blau --- Documentation/git-commit-graph.txt | 5 ++++- builtin/commit-graph.c | 13 +++++++++++-- 2 files changed, 15 insertions(+), 3 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 3edac318e8..93ff90d73b 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -34,6 +34,15 @@ static struct opts_commit_graph { int progress; } opts; +static struct object_directory *find_odb_or_die(struct repository *r, + const char *obj_dir) +{ + struct object_directory *odb = find_odb(r, obj_dir); + 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; @@ -78,7 +87,7 @@ static int graph_verify(int argc, const char **argv) graph = load_commit_graph_one_fd_st(fd, &st); else { struct object_directory *odb; - if ((odb = find_odb(the_repository, opts.obj_dir))) + if ((odb = find_odb_or_die(the_repository, opts.obj_dir))) graph = read_commit_graph_one(the_repository, odb); } @@ -149,7 +158,7 @@ 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); + odb = find_odb_or_die(the_repository, opts.obj_dir); if (opts.reachable) { if (write_commit_graph_reachable(odb->path, flags, &split_opts)) From patchwork Thu Jan 30 23:00:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 11358883 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 54C7A924 for ; Thu, 30 Jan 2020 23:00:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 2892E2063A for ; Thu, 30 Jan 2020 23:00:56 +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="cJRQBy34" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727810AbgA3XAz (ORCPT ); Thu, 30 Jan 2020 18:00:55 -0500 Received: from mail-pf1-f196.google.com ([209.85.210.196]:36883 "EHLO mail-pf1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727525AbgA3XAz (ORCPT ); Thu, 30 Jan 2020 18:00:55 -0500 Received: by mail-pf1-f196.google.com with SMTP id p14so2263846pfn.4 for ; Thu, 30 Jan 2020 15:00:54 -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=tgk0JrDIqvDuykRzCwC+WD+4tjEASuJ1j6oVKK/t4cs=; b=cJRQBy34vj7/Qy5SQXXuaY19InbVnxhq2Kfmrs91KL4I9NwM4EUKxbfpE83zzAvbtR EYSBra00pQnvGkSvW64N0wkAJBqwncnHA6A4vNF9mPp9+9E0dTSdgoxfFpZXPHq/kgxq IsTnULlbTz/EE9JSFYq94dQY36W7y6IfDza6VwRvpZ4oI0Zra+pGz3vhwgomTRhgF6Kn isPxk3XK8yWywhF6g+2TXiPZ6HWpZzQoS0CBdLz/HVx8aI9TtKFODF+bfNzbc0q5bZ7h yuDwlD9B1cu3HHVpkx96/pN8uriP9hkIVy7/LnzZotk8lN0XXZFWcPGLgf6OxfN3nDJo sYzw== 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=tgk0JrDIqvDuykRzCwC+WD+4tjEASuJ1j6oVKK/t4cs=; b=GZEZ/u/zAemi/WXlklRKFKnWJDNEAaSoQJqP7vPIZIH4Vm9AmrRXrLYgrKHNmd0lwA 273qndGp5MRkFvdKi4Lj/4BrdVJHouwJa/Ux6M71WYSGQhGxgLB33BcKW5bS8fDeE1bR X1aWJ91aFxQPPgZ3kv8dSUKX1fiFDoN5VteOvVaJYKDRtcdaNitOjNWEPsU6iSmEz3w+ hhPlannzdQheRSkfVZHYrrodukUv4m7FXU7cGqdJeHY/Jf5DW4uWP7KITm4pz6rNNTHS r3yxuNASM50O/L2bmVbj78gKIMfqimgeE8tF0w9zpI7DZZL2eODYZY58g8DRRJpMyjfW 39sg== X-Gm-Message-State: APjAAAVLRPK1GG+9xc0QSbcDVthN1lfeelr65XiYpa/Rflr8zrbbrs7D SLfOpvqsXcM/IHz7b/ioVn5M9Ik2gUOUog== X-Google-Smtp-Source: APXvYqzDQFGAigfUwffcaK05hzeLl9bDvULKjjTo/2lNdzPFL7vkVioBkbC7GjGdc3ssC6w55TJ2NA== X-Received: by 2002:a63:696:: with SMTP id 144mr7439336pgg.260.1580425253635; Thu, 30 Jan 2020 15:00:53 -0800 (PST) Received: from localhost ([2601:602:9200:32b0:5c8f:7dac:47b8:95ff]) by smtp.gmail.com with ESMTPSA id c6sm7751439pgk.78.2020.01.30.15.00.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Jan 2020 15:00:53 -0800 (PST) Date: Thu, 30 Jan 2020 15:00:52 -0800 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com Subject: [PATCH 4/6] commit-graph.h: store an odb in 'struct write_commit_graph_context' Message-ID: <3ddec36ba3f8929e5099fd9476de2cdfd06db4d7.1580424766.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 treatement to 'struct write_commit_graph_context', which is an 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 write_commit_graph_context' 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 | 4 ++-- builtin/commit.c | 2 +- builtin/fetch.c | 2 +- builtin/gc.c | 2 +- commit-graph.c | 42 ++++++++++++++++-------------------------- commit-graph.h | 4 ++-- 6 files changed, 23 insertions(+), 33 deletions(-) diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 93ff90d73b..9b1148a60a 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -161,7 +161,7 @@ static int graph_write(int argc, const char **argv) odb = find_odb_or_die(the_repository, opts.obj_dir); if (opts.reachable) { - if (write_commit_graph_reachable(odb->path, flags, &split_opts)) + if (write_commit_graph_reachable(odb, flags, &split_opts)) return 1; return 0; } @@ -183,7 +183,7 @@ static int graph_write(int argc, const char **argv) UNLEAK(buf); } - if (write_commit_graph(odb->path, + if (write_commit_graph(odb, pack_indexes, commit_hex, flags, diff --git a/builtin/commit.c b/builtin/commit.c index bd071169d7..9df3733e96 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 2c06876b26..19889e5fea 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -796,7 +796,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; @@ -1173,7 +1173,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) { @@ -1181,7 +1181,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); @@ -1196,7 +1196,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, @@ -1392,10 +1392,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)) { @@ -1406,7 +1406,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); @@ -1530,12 +1530,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; @@ -1577,7 +1577,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->obj_dir)) + if (strcmp(g->odb->path, ctx->odb->path)) break; num_commits += g->num_commits; @@ -1592,7 +1592,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) char *old_graph_name = get_commit_graph_filename(g->odb->path); if (!strcmp(g->filename, old_graph_name) && - strcmp(g->odb->path, ctx->obj_dir)) { + strcmp(g->odb->path, ctx->odb->path)) { ctx->num_commit_graphs_after = 1; ctx->new_base_graph = NULL; } @@ -1743,13 +1743,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); @@ -1788,7 +1788,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, @@ -1796,7 +1796,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)) @@ -1804,14 +1803,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; @@ -1848,8 +1840,7 @@ int write_commit_graph(const char *obj_dir, ctx->oids.alloc = split_opts->max_commits; if (ctx->append) { - struct object_directory *odb = find_odb(ctx->r, ctx->obj_dir); - prepare_commit_graph_one(ctx->r, odb); + prepare_commit_graph_one(ctx->r, ctx->odb); if (ctx->r->objects->commit_graph) ctx->oids.alloc += ctx->r->objects->commit_graph->num_commits; } @@ -1923,7 +1914,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 9700a6c7c2..39c0c0e51c 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -95,10 +95,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 Thu Jan 30 23:00:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 11358877 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 DAE88921 for ; Thu, 30 Jan 2020 23:00:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A4D2B214AF for ; Thu, 30 Jan 2020 23:00:49 +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="VqTXoycL" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727715AbgA3XAs (ORCPT ); Thu, 30 Jan 2020 18:00:48 -0500 Received: from mail-pg1-f193.google.com ([209.85.215.193]:39804 "EHLO mail-pg1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727566AbgA3XAs (ORCPT ); Thu, 30 Jan 2020 18:00:48 -0500 Received: by mail-pg1-f193.google.com with SMTP id 4so2416978pgd.6 for ; Thu, 30 Jan 2020 15:00:47 -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=6hm1RlqJikIrE4eWRdQvIwRIQZt5NItXyJqayitApU4=; b=VqTXoycLxhn2s6y77OAstO4zO8EPA4D6foEnZn4J9iOmkfq96GhTw9LZiXfLvO7pnp fNWGRz+hFEQuOl0hrXbDCOMbv8nS27IeENX1Iuhfh6VrZ1p29ZJxEG6hZJgGaPtK15SZ l0hB6pASKeV8p9aXepPKTrfY9kxyaxXoxx9eaAkJUSXMzy/N7RMShZR3erVTI2AQZ9XU tfZayRfnhaxXQwVcBEL6z3DwurI4mcOWHbt+eFUj3wjgCdLinUkDtW4QCsGsPpySL25H zLOo5JYVLdOkHoFzCE3cOoFCPxzMfl9XBwLRKk3DdMsajbrSS+MttOZqCqfaTz1pUzn4 +c1g== 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=6hm1RlqJikIrE4eWRdQvIwRIQZt5NItXyJqayitApU4=; b=I8vvbrK/ughePRBMZb3yi6UFNnxG6ONvAyNnDTRjRXUuzeiCWhj2tfX7OinNC/r76p CFKHxCmL6NTYpin++S55x1ebkXu35/y4Pk6tq/54ajE2OJ8mzcGRO21GYWGt5twnuroj qZQ3Tzsp95PsguOM6HqrNgA0uXv2r6QJ5cm7wBYambKCbYdt0e7GpijDCC/68Xqu0RDV YNReQnUitundmW2Ijkfd8bDK5GVhUPy+tTWozYAP65r/3JtObx+Oi5VxE6k02/KjkLC1 4EFIgUUP1C2XUkpnW7xNwEC1PvWAXFz9lhEoXJe1pBTvoS85xR7Dj3lVJqA+U576s8Pg 5B0Q== X-Gm-Message-State: APjAAAVNI/yN1TxLyD5fns51JZZKaekbk3VbBCv95SkDbJ1xis0nYdl2 rIOk8EOTasny9jHGg0E7a7cvGXzZO8dORQ== X-Google-Smtp-Source: APXvYqxq2wVJfvzN0beS65j5bkuyXf0BNwZkXD2O3uafna6LB+vKmXZVLfoZ3S9Da/0BLa7fivOGrg== X-Received: by 2002:a62:3603:: with SMTP id d3mr6941179pfa.37.1580425246911; Thu, 30 Jan 2020 15:00:46 -0800 (PST) Received: from localhost ([2601:602:9200:32b0:5c8f:7dac:47b8:95ff]) by smtp.gmail.com with ESMTPSA id s22sm7410235pji.30.2020.01.30.15.00.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Jan 2020 15:00:46 -0800 (PST) Date: Thu, 30 Jan 2020 15:00:45 -0800 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com Subject: [PATCH 5/6] 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 | 11 ++++----- commit-graph.c | 47 +++++++++++++++----------------------- commit-graph.h | 2 +- t/helper/test-read-graph.c | 6 ++--- 4 files changed, 28 insertions(+), 38 deletions(-) diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 9b1148a60a..4ab045395e 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -46,6 +46,7 @@ static struct object_directory *find_odb_or_die(struct repository *r, 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; @@ -76,7 +77,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_or_die(the_repository, opts.obj_dir); + 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); @@ -85,11 +87,8 @@ static int graph_verify(int argc, const char **argv) if (open_ok) graph = load_commit_graph_one_fd_st(fd, &st); - else { - struct object_directory *odb; - if ((odb = find_odb_or_die(the_repository, opts.obj_dir))) - graph = read_commit_graph_one(the_repository, odb); - } + else + 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 19889e5fea..09316240f0 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) @@ -350,7 +341,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); @@ -401,7 +392,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; @@ -434,7 +425,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); @@ -1395,7 +1386,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)) { @@ -1406,7 +1397,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); @@ -1494,7 +1485,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]); @@ -1530,12 +1521,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; @@ -1577,7 +1568,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; @@ -1589,10 +1580,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; } @@ -1743,7 +1734,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 39c0c0e51c..97e2bce313 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); struct object_directory *find_odb(struct repository *r, const char *obj_dir); 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 Thu Jan 30 23:00:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 11358879 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 02D37921 for ; Thu, 30 Jan 2020 23:00:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D65A6214AF for ; Thu, 30 Jan 2020 23:00:51 +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="TsnvYCrV" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727685AbgA3XAv (ORCPT ); Thu, 30 Jan 2020 18:00:51 -0500 Received: from mail-pg1-f194.google.com ([209.85.215.194]:42615 "EHLO mail-pg1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727525AbgA3XAu (ORCPT ); Thu, 30 Jan 2020 18:00:50 -0500 Received: by mail-pg1-f194.google.com with SMTP id s64so2405043pgb.9 for ; Thu, 30 Jan 2020 15:00:49 -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=q6/xtevWMepwUgTxVi+MaHnClkd9BUixgXtTuS1NRis=; b=TsnvYCrV4ADNX9l7iGE/OzqG348SonxXSM7jDliqFy1x8rPEeXfIymlR8E76L701Ya NcFzT8YHjYAiePEBhVUcx/Gcf9AAxwYR9+1qr3I+eZADDqlFzfNDMKNRbtKVb4EZhJUO osE+KAyHUe0QGANIogh/k7amyRfMeNGXknbLxXChWaj09gCXlvm2AOlx9Cebvv+7Xmnb fDl5CPUNg1UkPi71FQTvj59uSlXvGNXze8ZMWev4uZOZ6qlULCR96uj/z3D5rlUFL5lM gDgPHjfGEZyVZDXihcP9ndRto6dSoqRRUgt1clBUNhO4+RGzGPZEffFDByVNnwUJQuxn vGrA== 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=q6/xtevWMepwUgTxVi+MaHnClkd9BUixgXtTuS1NRis=; b=r51JtEV1BiCoQEmrgm6Cmp2LsJjbLHESKtbbNj2F2Px2PmYue+5XdjsHKoszNR64Yt 7maYKQzcBPF9knmiD1a61/fcxahh5cUYb7wAHxGcCI0rEFDd7rpuvTB1orb57nBOioM9 PAYQOzy9e8E5lACOzLej/2Ije9Brkx0iTE80inSfrWsaIjWPt1MWYxSFhSkSpRD+Quf4 JqYlpxUNp96abh15gcsqD6LGG5nheOOQDo4I52ZVHIFTdm4jxiItVaLumb0gVHc6GVKo Jb0JzonCFkUtN5nUQZdU3aLr4IxvjndFwLCvd53bF6jNmZN9uorPsEt+MBgwsExGhPga o1xA== X-Gm-Message-State: APjAAAXqfqkBVn7scWFy4pubhOlFA5XKOjYkkDnLHvJzB0QS0vs77vaG 1fAC/jYzRAtvlTaikQdHnUfekHpL8Xcwuw== X-Google-Smtp-Source: APXvYqwHXmQ/IP9pJOQZ6NUsK+uwBOiMIVwWYzkVqNVL7g0hudN+tv1oykXyz2KCwMV5dzyk1ewgNQ== X-Received: by 2002:a63:211f:: with SMTP id h31mr6678092pgh.299.1580425249034; Thu, 30 Jan 2020 15:00:49 -0800 (PST) Received: from localhost ([2601:602:9200:32b0:5c8f:7dac:47b8:95ff]) by smtp.gmail.com with ESMTPSA id m101sm7827422pje.13.2020.01.30.15.00.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Jan 2020 15:00:48 -0800 (PST) Date: Thu, 30 Jan 2020 15:00:47 -0800 From: Taylor Blau To: git@vger.kernel.org Cc: peff@peff.net, dstolee@microsoft.com, gitster@pobox.com Subject: [PATCH 6/6] commit-graph.h: use odb in 'load_commit_graph_one_fd_st' Message-ID: <15c74d0f65545d7ceb258712e27c388b58cd54d9.1580424766.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 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 4ab045395e..de321c71ad 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -86,7 +86,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 09316240f0..6d34829f57 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -128,7 +128,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; @@ -144,7 +145,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); } @@ -319,7 +322,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; @@ -330,7 +334,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); @@ -342,12 +346,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; } @@ -426,13 +427,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 97e2bce313..7d9fc9c16a 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -63,7 +63,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;