grep: allocate subrepos on heap

Currently, struct repository objects corresponding to submodules are allocated on the stack in grep_submodule(). This currently works because they will not be used once grep_submodule() exits, but a subsequent patch will require these structs to be accessible for longer (perhaps even in another thread). Allocate them on the heap and clear them only at the very end. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Reviewed-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Aug 16, 2021 at 14:09 UTC dd45471a3717bcd6561e405371b81928214ad1b5
1 file changed +30 -9
builtin/grep.c
+30 -9
@@ -65,6 +65,9 @@ static int todo_done;
65 /* Has all work items been added? */
66 static int all_work_added;
67
68 +static struct repository **repos_to_free;
69 +static size_t repos_to_free_nr, repos_to_free_alloc;
70 +
71 /* This lock protects all the variables above. */
72 static pthread_mutex_t grep_mutex;
73
@@ -168,6 +171,19 @@ static void work_done(struct work_item *w)
171 grep_unlock();
172 }
173
174 +static void free_repos(void)
175 +{
176 + int i;
177 +
178 + for (i = 0; i < repos_to_free_nr; i++) {
179 + repo_clear(repos_to_free[i]);
180 + free(repos_to_free[i]);
181 + }
182 + FREE_AND_NULL(repos_to_free);
183 + repos_to_free_nr = 0;
184 + repos_to_free_alloc = 0;
185 +}
186 +
187 static void *run(void *arg)
188 {
189 int hit = 0;
@@ -415,19 +431,24 @@ static int grep_submodule(struct grep_opt *opt,
431 const struct object_id *oid,
432 const char *filename, const char *path, int cached)
433 {
418 - struct repository subrepo;
434 + struct repository *subrepo;
435 struct repository *superproject = opt->repo;
436 const struct submodule *sub;
437 struct grep_opt subopt;
422 - int hit;
438 + int hit = 0;
439
440 sub = submodule_from_path(superproject, null_oid(), path);
441
442 if (!is_submodule_active(superproject, path))
443 return 0;
444
429 - if (repo_submodule_init(&subrepo, superproject, sub))
445 + subrepo = xmalloc(sizeof(*subrepo));
446 + if (repo_submodule_init(subrepo, superproject, sub)) {
447 + free(subrepo);
448 return 0;
449 + }
450 + ALLOC_GROW(repos_to_free, repos_to_free_nr + 1, repos_to_free_alloc);
451 + repos_to_free[repos_to_free_nr++] = subrepo;
452
453 /*
454 * NEEDSWORK: repo_read_gitmodules() might call
@@ -438,7 +459,7 @@ static int grep_submodule(struct grep_opt *opt,
459 * subrepo's odbs to the in-memory alternates list.
460 */
461 obj_read_lock();
441 - repo_read_gitmodules(&subrepo, 0);
462 + repo_read_gitmodules(subrepo, 0);
463
464 /*
465 * NEEDSWORK: This adds the submodule's object directory to the list of
@@ -450,11 +471,11 @@ static int grep_submodule(struct grep_opt *opt,
471 * store is no longer global and instead is a member of the repository
472 * object.
473 */
453 - add_submodule_odb_by_path(subrepo.objects->odb->path);
474 + add_submodule_odb_by_path(subrepo->objects->odb->path);
475 obj_read_unlock();
476
477 memcpy(&subopt, opt, sizeof(subopt));
457 - subopt.repo = &subrepo;
478 + subopt.repo = subrepo;
479
480 if (oid) {
481 enum object_type object_type;
@@ -464,9 +485,9 @@ static int grep_submodule(struct grep_opt *opt,
485 struct strbuf base = STRBUF_INIT;
486
487 obj_read_lock();
467 - object_type = oid_object_info(&subrepo, oid, NULL);
488 + object_type = oid_object_info(subrepo, oid, NULL);
489 obj_read_unlock();
469 - data = read_object_with_reference(&subrepo,
490 + data = read_object_with_reference(subrepo,
491 oid, tree_type,
492 &size, NULL);
493 if (!data)
@@ -484,7 +505,6 @@ static int grep_submodule(struct grep_opt *opt,
505 hit = grep_cache(&subopt, pathspec, cached);
506 }
507
487 - repo_clear(&subrepo);
508 return hit;
509 }
510
@@ -1182,5 +1202,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
1202 run_pager(&opt, prefix);
1203 clear_pathspec(&pathspec);
1204 free_grep_patterns(&opt);
1205 + free_repos();
1206 return !hit;
1207 }