Thread (1 message) 1 message, 1 author, 2021-11-11

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help