From 4468d4435c44d50723e96e3416f8b5da97a1806f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Sun, 27 Jan 2019 14:08:32 +0100 Subject: [PATCH] object_as_type: initialize commit-graph-related fields of 'struct commit' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the commit graph and generation numbers were introduced in commits 177722b344 (commit: integrate commit graph with commit parsing, 2018-04-10) and 83073cc994 (commit: add generation number to struct commit, 2018-04-25), they tried to make sure that the corresponding 'graph_pos' and 'generation' fields of 'struct commit' are initialized conservatively, as if the commit were not included in the commit-graph file. Alas, initializing those fields only in alloc_commit_node() missed the case when an object that happens to be a commit is first looked up via lookup_unknown_object(), and is then later converted to a 'struct commit' via the object_as_type() helper function (either calling it directly, or as part of a subsequent lookup_commit() call). Consequently, both of those fields incorrectly remain set to zero, which means e.g. that the commit is present in and is the first entry of the commit-graph file. This will result in wrong timestamp, parent and root tree hashes, if such a 'struct commit' instance is later filled from the commit-graph. Extract the initialization of 'struct commit's fields from alloc_commit_node() into a helper function, and call it from object_as_type() as well, to make sure that it properly initializes the two commit-graph-related fields, too. With this helper function it is hopefully less likely that any new fields added to 'struct commit' in the future would remain uninitialized. With this change alloc_commit_index() won't have any remaining callers outside of 'alloc.c', so mark it as static. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- alloc.c | 11 ++++++++--- alloc.h | 2 +- object.c | 5 +++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/alloc.c b/alloc.c index e7aa81b7aa..1c64c4dd16 100644 --- a/alloc.c +++ b/alloc.c @@ -99,18 +99,23 @@ void *alloc_object_node(struct repository *r) return obj; } -unsigned int alloc_commit_index(struct repository *r) +static unsigned int alloc_commit_index(struct repository *r) { return r->parsed_objects->commit_count++; } -void *alloc_commit_node(struct repository *r) +void init_commit_node(struct repository *r, struct commit *c) { - struct commit *c = alloc_node(r->parsed_objects->commit_state, sizeof(struct commit)); c->object.type = OBJ_COMMIT; c->index = alloc_commit_index(r); c->graph_pos = COMMIT_NOT_FROM_GRAPH; c->generation = GENERATION_NUMBER_INFINITY; +} + +void *alloc_commit_node(struct repository *r) +{ + struct commit *c = alloc_node(r->parsed_objects->commit_state, sizeof(struct commit)); + init_commit_node(r, c); return c; } diff --git a/alloc.h b/alloc.h index ba356ed847..ed1071c11e 100644 --- a/alloc.h +++ b/alloc.h @@ -9,11 +9,11 @@ struct repository; void *alloc_blob_node(struct repository *r); void *alloc_tree_node(struct repository *r); +void init_commit_node(struct repository *r, struct commit *c); void *alloc_commit_node(struct repository *r); void *alloc_tag_node(struct repository *r); void *alloc_object_node(struct repository *r); void alloc_report(struct repository *r); -unsigned int alloc_commit_index(struct repository *r); struct alloc_state *allocate_alloc_state(void); void clear_alloc_state(struct alloc_state *s); diff --git a/object.c b/object.c index 51c4594515..675cb8902b 100644 --- a/object.c +++ b/object.c @@ -164,8 +164,9 @@ void *object_as_type(struct repository *r, struct object *obj, enum object_type return obj; else if (obj->type == OBJ_NONE) { if (type == OBJ_COMMIT) - ((struct commit *)obj)->index = alloc_commit_index(r); - obj->type = type; + init_commit_node(r, (struct commit *) obj); + else + obj->type = type; return obj; } else {