Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> 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.
