Am 26.08.19 um 19:20 schrieb Junio C Hamano: > Stefan Sperling <stsp@xxxxxxxxx> writes: > >> The root cause of this bug seems to be that the valid assumption >> that obj->parsed implies a successfully parsed object is broken by >> parse_tag_buffer() because this function sets the 'parsed' flag even >> if errors occur during parsing. > > I am mildly negative about that approach. obj->parsed is about > "we've done all we need to do to attempt parsing this object" (so > that next person who gets hold of the object knows that fact---one > of the reasons why may be that the caller who wants to ensure that > the fields are ready to be accessed does not have to spend extra > cycles, but that is not the only one). Those that want to look at > various fields in the object (e.g. the tagged object of a tag, the > tagger identity of a tag, etc.) should be prepared to see and react > to NULL in there so that they can gracefully handle "slightly" > corrupt objects. Not sure how this could happen under normal circumstances, but how about this here? -- >8 -- Subject: [PATCH] tree: simplify parse_tree_indirect() Reduce code duplication by turning parse_tree_indirect() into a wrapper of repo_peel_to_type(). This avoids a segfault when handling a broken tag where ->tagged is NULL. The new version also checks the return value of parse_object() that was ignored by the old one. Initial-patch-by: Stefan Sperling <stsp@xxxxxxxxx> Signed-off-by: René Scharfe <l.s.r@xxxxxx> --- tree.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/tree.c b/tree.c index 4720945e6a..1466bcc6a8 100644 --- a/tree.c +++ b/tree.c @@ -244,19 +244,7 @@ void free_tree_buffer(struct tree *tree) struct tree *parse_tree_indirect(const struct object_id *oid) { - struct object *obj = parse_object(the_repository, oid); - do { - if (!obj) - return NULL; - if (obj->type == OBJ_TREE) - return (struct tree *) obj; - else if (obj->type == OBJ_COMMIT) - obj = &(get_commit_tree(((struct commit *)obj))->object); - else if (obj->type == OBJ_TAG) - obj = ((struct tag *) obj)->tagged; - else - return NULL; - if (!obj->parsed) - parse_object(the_repository, &obj->oid); - } while (1); + struct repository *r = the_repository; + struct object *obj = parse_object(r, oid); + return (struct tree *)repo_peel_to_type(r, NULL, 0, obj, OBJ_TREE); } -- 2.23.0