object_array: use object_array_clear(), not free()

Martin Ågren · gitster · commit dcb572ab94f8 · 2017-09-24T10:06:01.000+09:00
Instead of freeing `foo.objects` for an object array `foo` (sometimes
conditionally), call `object_array_clear(&amp;foo)`. This means we don't
poke as much into the implementation, which is already a good thing, but
also that we release the individual entries as well, thereby fixing at
least one memory-leak (in diff-lib.c).

If someone is holding on to a pointer to an element's `name` or `path`,
that is now a dangling pointer, i.e., we'd be turning an unpleasant
situation into an outright bug. To the best of my understanding no such
long-term pointers are being taken.

The way we handle `study` in builting/reflog.c still looks like it might
leak. That will be addressed in the next commit.

Signed-off-by: Martin Ågren &lt;martin.agren@gmail.com&gt;
Reviewed-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/builtin/reflog.c b/builtin/reflog.c
@@ -182,8 +182,8 @@ static int commit_is_complete(struct commit *commit)
 			found.objects[i].item->flags |= SEEN;
 	}
 	/* free object arrays */
-	free(study.objects);
-	free(found.objects);
+	object_array_clear(&study);
+	object_array_clear(&found);
 	return !is_incomplete;
 }
 
diff --git a/diff-lib.c b/diff-lib.c
@@ -549,7 +549,6 @@ int index_differs_from(const char *def, int diff_flags,
 	rev.diffopt.flags |= diff_flags;
 	rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
 	run_diff_index(&rev, 1);
-	if (rev.pending.alloc)
-		free(rev.pending.objects);
+	object_array_clear(&rev.pending);
 	return (DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0);
 }
diff --git a/submodule.c b/submodule.c
@@ -1728,7 +1728,7 @@ static int find_first_merges(struct object_array *result, const char *path,
 			add_object_array(merges.objects[i].item, NULL, result);
 	}
 
-	free(merges.objects);
+	object_array_clear(&merges);
 	return result->nr;
 }
 
@@ -1833,7 +1833,7 @@ int merge_submodule(struct object_id *result, const char *path,
 			print_commit((struct commit *) merges.objects[i].item);
 	}
 
-	free(merges.objects);
+	object_array_clear(&merges);
 	return 0;
 }
 
diff --git a/upload-pack.c b/upload-pack.c
@@ -888,7 +888,7 @@ static void receive_needs(void)
 		}
 
 	shallow_nr += shallows.nr;
-	free(shallows.objects);
+	object_array_clear(&shallows);
 }
 
 /* return non-zero if the ref is hidden, otherwise 0 */

Original file line number	Diff line number	Diff line change
`@@ -182,8 +182,8 @@ static int commit_is_complete(struct commit *commit)`
`182`	`182`	`found.objects[i].item->flags \|= SEEN;`
`183`	`183`	`}`
`184`	`184`	`/* free object arrays */`
`185`		`- free(study.objects);`
`186`		`- free(found.objects);`
	`185`	`+ object_array_clear(&study);`
	`186`	`+ object_array_clear(&found);`
`187`	`187`	`return !is_incomplete;`
`188`	`188`	`}`
`189`	`189`
Original file line number	Diff line number	Diff line change
`@@ -549,7 +549,6 @@ int index_differs_from(const char *def, int diff_flags,`
`549`	`549`	`rev.diffopt.flags \|= diff_flags;`
`550`	`550`	`rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;`
`551`	`551`	`run_diff_index(&rev, 1);`
`552`		`- if (rev.pending.alloc)`
`553`		`- free(rev.pending.objects);`
	`552`	`+ object_array_clear(&rev.pending);`
`554`	`553`	`return (DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0);`
`555`	`554`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1728,7 +1728,7 @@ static int find_first_merges(struct object_array result, const char path,`
`1728`	`1728`	`add_object_array(merges.objects[i].item, NULL, result);`
`1729`	`1729`	`}`
`1730`	`1730`
`1731`		`- free(merges.objects);`
	`1731`	`+ object_array_clear(&merges);`
`1732`	`1732`	`return result->nr;`
`1733`	`1733`	`}`
`1734`	`1734`
`@@ -1833,7 +1833,7 @@ int merge_submodule(struct object_id result, const char path,`
`1833`	`1833`	`print_commit((struct commit *) merges.objects[i].item);`
`1834`	`1834`	`}`
`1835`	`1835`
`1836`		`- free(merges.objects);`
	`1836`	`+ object_array_clear(&merges);`
`1837`	`1837`	`return 0;`
`1838`	`1838`	`}`
`1839`	`1839`
Original file line number	Diff line number	Diff line change
`@@ -888,7 +888,7 @@ static void receive_needs(void)`
`888`	`888`	`}`
`889`	`889`
`890`	`890`	`shallow_nr += shallows.nr;`
`891`		`- free(shallows.objects);`
	`891`	`+ object_array_clear(&shallows);`
`892`	`892`	`}`
`893`	`893`
`894`	`894`	`/* return non-zero if the ref is hidden, otherwise 0 */`