Re: [PATCH 2/2] object-file: free(*contents) only in read_loose_object() caller
From: Junio C Hamano <hidden>
Date: 2021-11-11 18:54:25
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted hunk
diff --git a/builtin/fsck.c b/builtin/fsck.c index d87c28a1cc4..27b9e78094d 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c@@ -605,7 +605,7 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *data) struct object *obj; enum object_type type = OBJ_NONE; unsigned long size; - void *contents; + void *contents = NULL; int eaten; struct object_info oi = OBJECT_INFO_INIT; struct object_id real_oid = *null_oid();@@ -630,6 +630,7 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *data) path); if (err < 0) { errors_found |= ERROR_OBJECT; + free(contents); return 0; /* keep checking other objects */ }diff --git a/object-file.c b/object-file.c index ac476653a06..c3d866a287e 100644 --- a/object-file.c +++ b/object-file.c@@ -2528,8 +2528,6 @@ int read_loose_object(const char *path, char hdr[MAX_HEADER_LEN]; unsigned long *size = oi->sizep; - *contents = NULL; - map = map_loose_object_1(the_repository, path, NULL, &mapsize); if (!map) { error_errno(_("unable to mmap %s"), path);@@ -2559,10 +2557,9 @@ int read_loose_object(const char *path, goto out; } if (check_object_signature(the_repository, expected_oid, - *contents, *size, oi->type_name->buf, real_oid)) { - free(*contents); + *contents, *size, + oi->type_name->buf, real_oid)) goto out; - } }
Yeah, I have to say that read_loose_object() that frees *contents without clearing *contents to NULL only because it wants to signal if the failure comes from check_object_signature() step is quite ugly. Making the caller responsible for freeing (in other words, when caller's *contents is non-NULL after function returns, it always has a valid piece of memory to be freed) makes the contract easier to explain.