From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Derrick Stolee" <stolee@gmail.com>,
"SZEDER Gábor" <szeder.dev@gmail.com>,
"Eric Sunshine" <sunshine@sunshineco.com>,
"Ramsay Jones" <ramsay@ramsayjones.plus.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v3 0/8] commit-graph: segfault & other fixes for broken graphs
Date: Mon, 25 Mar 2019 13:08:26 +0100 [thread overview]
Message-ID: <20190325120834.15529-1-avarab@gmail.com> (raw)
In-Reply-To: <20190314214740.23360-1-avarab@gmail.com>
This v3 fixes issues raised by Ramsay in
https://public-inbox.org/git/1908832c-8dd0-377e-917b-acb33b00273c@ramsayjones.plus.com/
While I was at it I changed a die("Whatevs") to
die("whatevs"). I.e. start with lower-case as discussed in other
places on-list recently.
The range-diff looks scarier than this v2..v3 really is. There's no
code changes aside from the s/0/NULL/ in one place, but marking a
couple of functions as "static" required moving them around, and
removing their entry from the corresponding *.h file.
Ævar Arnfjörð Bjarmason (8):
commit-graph tests: split up corrupt_graph_and_verify()
commit-graph tests: test a graph that's too small
commit-graph: fix segfault on e.g. "git status"
commit-graph: don't early exit(1) on e.g. "git status"
commit-graph: don't pass filename to load_commit_graph_one_fd_st()
commit-graph verify: detect inability to read the graph
commit-graph write: don't die if the existing graph is corrupt
commit-graph: improve & i18n error messages
builtin/commit-graph.c | 23 +++++--
commit-graph.c | 132 +++++++++++++++++++++++++++-------------
commit-graph.h | 4 +-
commit.h | 6 ++
t/t5318-commit-graph.sh | 42 +++++++++++--
5 files changed, 153 insertions(+), 54 deletions(-)
Range-diff:
1: 2f8ba0adf8 = 1: 83ff92a39d commit-graph tests: split up corrupt_graph_and_verify()
2: 800b17edde = 2: b9170c35e6 commit-graph tests: test a graph that's too small
3: 7083ab81c7 ! 3: daf38a9af7 commit-graph: fix segfault on e.g. "git status"
@@ -58,20 +58,10 @@
--- a/commit-graph.c
+++ b/commit-graph.c
@@
- last_chunk_offset = chunk_offset;
- }
-
-+ if (verify_commit_graph_lite(graph))
-+ return NULL;
-+
- return graph;
+ return ret;
}
-@@
- #define GENERATION_ZERO_EXISTS 1
- #define GENERATION_NUMBER_EXISTS 2
-
-+int verify_commit_graph_lite(struct commit_graph *g)
++static int verify_commit_graph_lite(struct commit_graph *g)
+{
+ /*
+ * Basic validation shared between parse_commit_graph()
@@ -101,9 +91,19 @@
+ return 0;
+}
+
- int verify_commit_graph(struct repository *r, struct commit_graph *g)
+ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
+ size_t graph_size)
{
- uint32_t i, cur_fanout_pos = 0;
+@@
+ last_chunk_offset = chunk_offset;
+ }
+
++ if (verify_commit_graph_lite(graph))
++ return NULL;
++
+ return graph;
+ }
+
@@
return 1;
}
@@ -122,18 +122,6 @@
return verify_commit_graph_error;
- diff --git a/commit-graph.h b/commit-graph.h
- --- a/commit-graph.h
- +++ b/commit-graph.h
-@@
- struct string_list *commit_hex,
- int append, int report_progress);
-
-+int verify_commit_graph_lite(struct commit_graph *g);
- int verify_commit_graph(struct repository *r, struct commit_graph *g);
-
- void close_commit_graph(struct repository *);
-
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
4: d00564ae89 ! 4: 3d7d8c4deb commit-graph: don't early exit(1) on e.g. "git status"
@@ -29,7 +29,15 @@
passed to that function for the the "graph file %s is too small" error
message.
+ This leaves load_commit_graph_one() unused by everything except the
+ internal prepare_commit_graph_one() function, so let's mark it as
+ "static". If someone needs it in the future we can remove the "static"
+ attribute. I could also rewrite its sole remaining
+ user ("prepare_commit_graph_one()") to use
+ load_commit_graph_one_fd_st() instead, but let's leave it at this.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
+ Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
--- a/builtin/commit-graph.c
@@ -131,7 +139,7 @@
close(fd);
- die(_("graph file %s is too small"), graph_file);
+ error(_("graph file %s is too small"), graph_file);
-+ return 0;
++ return NULL;
}
graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
ret = parse_commit_graph(graph_map, fd, graph_size);
@@ -143,9 +151,11 @@
}
return ret;
+@@
+ return graph;
}
-+struct commit_graph *load_commit_graph_one(const char *graph_file)
++static struct commit_graph *load_commit_graph_one(const char *graph_file)
+{
+
+ struct stat st;
@@ -158,9 +168,9 @@
+ return load_commit_graph_one_fd_st(graph_file, fd, &st);
+}
+
- struct commit_graph *parse_commit_graph(void *graph_map, int fd,
- size_t graph_size)
+ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
{
+ char *graph_name;
diff --git a/commit-graph.h b/commit-graph.h
--- a/commit-graph.h
@@ -174,9 +184,10 @@
/*
* Given a commit struct, try to fill the commit struct info, including:
@@
+ const unsigned char *chunk_extra_edges;
};
- struct commit_graph *load_commit_graph_one(const char *graph_file);
+-struct commit_graph *load_commit_graph_one(const char *graph_file);
+struct commit_graph *load_commit_graph_one_fd_st(const char *graph_file,
+ int fd, struct stat *st);
5: 25ee185bf7 ! 5: 15687fb21f commit-graph: don't pass filename to load_commit_graph_one_fd_st()
@@ -59,7 +59,7 @@
close(fd);
- error(_("graph file %s is too small"), graph_file);
+ error(_("commit-graph file is too small"));
- return 0;
+ return NULL;
}
graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
@@
@@ -70,15 +70,15 @@
+ return load_commit_graph_one_fd_st(fd, &st);
}
- struct commit_graph *parse_commit_graph(void *graph_map, int fd,
+ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
diff --git a/commit-graph.h b/commit-graph.h
--- a/commit-graph.h
+++ b/commit-graph.h
@@
+ const unsigned char *chunk_extra_edges;
};
- struct commit_graph *load_commit_graph_one(const char *graph_file);
-struct commit_graph *load_commit_graph_one_fd_st(const char *graph_file,
- int fd, struct stat *st);
+struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st);
6: 7619b46987 = 6: 30145c2dca commit-graph verify: detect inability to read the graph
7: 17ee4fc050 ! 7: 51e3aa651b commit-graph write: don't die if the existing graph is corrupt
@@ -57,7 +57,7 @@
int config_value;
+ if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
-+ die("Dying as requested by the '%s' variable on commit-graph load!",
++ die("dying as requested by the '%s' variable on commit-graph load!",
+ GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD);
+
if (r->objects->commit_graph_attempted)
8: 29ab2895b7 = 8: e7c801f73b commit-graph: improve & i18n error messages
--
2.21.0.360.g471c308f928
next prev parent reply other threads:[~2019-03-25 12:08 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-21 22:37 [PATCH 0/8] commit-graph: segfault & other fixes for broken graphs Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 1/8] commit-graph tests: split up corrupt_graph_and_verify() Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 2/8] commit-graph tests: test a graph that's too small Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 3/8] commit-graph: fix segfault on e.g. "git status" Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 4/8] commit-graph: don't early exit(1) " Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 5/8] commit-graph: don't pass filename to load_commit_graph_one_fd_st() Ævar Arnfjörð Bjarmason
2019-02-21 22:37 ` [PATCH 6/8] commit-graph verify: detect inability to read the graph Ævar Arnfjörð Bjarmason
2019-02-21 23:15 ` SZEDER Gábor
2019-02-21 22:37 ` [PATCH 7/8] commit-graph write: don't die if the existing graph is corrupt Ævar Arnfjörð Bjarmason
2019-02-22 23:13 ` Junio C Hamano
2019-02-23 9:29 ` Eric Sunshine
2019-02-21 22:37 ` [PATCH 8/8] commit-graph: improve & i18n error messages Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 0/8] commit-graph: segfault & other fixes for broken graphs Ævar Arnfjörð Bjarmason
2019-03-15 7:47 ` Eric Sunshine
2019-03-15 10:39 ` Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` Ævar Arnfjörð Bjarmason [this message]
2019-03-25 12:08 ` [PATCH v3 1/8] commit-graph tests: split up corrupt_graph_and_verify() Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 2/8] commit-graph tests: test a graph that's too small Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 3/8] commit-graph: fix segfault on e.g. "git status" Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 4/8] commit-graph: don't early exit(1) " Ævar Arnfjörð Bjarmason
2019-04-27 13:06 ` Ævar Arnfjörð Bjarmason
2019-04-29 12:48 ` Derrick Stolee
2019-04-29 14:30 ` Ævar Arnfjörð Bjarmason
2019-05-01 18:31 ` Jeff King
2019-03-25 12:08 ` [PATCH v3 5/8] commit-graph: don't pass filename to load_commit_graph_one_fd_st() Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 6/8] commit-graph verify: detect inability to read the graph Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 7/8] commit-graph write: don't die if the existing graph is corrupt Ævar Arnfjörð Bjarmason
2019-03-25 12:08 ` [PATCH v3 8/8] commit-graph: improve & i18n error messages Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 1/8] commit-graph tests: split up corrupt_graph_and_verify() Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 2/8] commit-graph tests: test a graph that's too small Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 3/8] commit-graph: fix segfault on e.g. "git status" Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 4/8] commit-graph: don't early exit(1) " Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 5/8] commit-graph: don't pass filename to load_commit_graph_one_fd_st() Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 6/8] commit-graph verify: detect inability to read the graph Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 7/8] commit-graph write: don't die if the existing graph is corrupt Ævar Arnfjörð Bjarmason
2019-03-14 21:47 ` [PATCH v2 8/8] commit-graph: improve & i18n error messages Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190325120834.15529-1-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=ramsay@ramsayjones.plus.com \
--cc=stolee@gmail.com \
--cc=sunshine@sunshineco.com \
--cc=szeder.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).