Download raw body.
gotwebd: plug memleaks in got_get_repo_commits
second attemp at fixing this. For my own mental sanity and for who will read this, I've split it into three separate commits to ease reviewing. The first two should be evident, the third one is a bit more convoluted: it avoids calling two times got_get_repo_commit (singular!) and leaking at every iteration the contents of `repo_commit'. Instead, drop the new_repo_commit var and only use repo_commit. To ease tracking the lifetime of it, immediately push it into the tailq like got_get_repo_tags does. ok? ----------------------------------------------- commit 548422fdbb511de35fb644b9e8e5c6effbba76da from: Omar Polo <op@omarpolo.com> date: Fri Sep 2 10:42:30 2022 UTC gotwebd: always free ref in got_get_repo_commits some code-paths may leak it. diff bc95141ca7ce90e4b19a251b36c87601c150bb3f 548422fdbb511de35fb644b9e8e5c6effbba76da commit - bc95141ca7ce90e4b19a251b36c87601c150bb3f commit + 548422fdbb511de35fb644b9e8e5c6effbba76da blob - 4beb308778696b4c4299304065d186d28b072d3e blob + dcc24dae3dae23f69341a447788cdf594fc9a613 --- gotwebd/got_operations.c +++ gotwebd/got_operations.c @@ -331,7 +331,7 @@ got_get_repo_commits(struct request *c, int limit) struct got_commit_graph *graph = NULL; struct got_commit_object *commit = NULL; struct got_reflist_head refs; - struct got_reference *ref; + struct got_reference *ref = NULL; struct repo_commit *repo_commit = NULL; struct server *srv = c->srv; struct transport *t = c->t; @@ -373,7 +373,6 @@ got_get_repo_commits(struct request *c, int limit) goto done; error = got_ref_resolve(&id, repo, ref); - got_ref_close(ref); if (error) goto done; } else if (qs->commit != NULL) { @@ -383,7 +382,6 @@ got_get_repo_commits(struct request *c, int limit) if (error) goto done; error = got_object_get_type(&obj_type, repo, id); - got_ref_close(ref); if (error) goto done; if (obj_type == GOT_OBJ_TYPE_TAG) { @@ -545,6 +543,8 @@ got_get_repo_commits(struct request *c, int limit) } done: gotweb_free_repo_commit(repo_commit); + if (ref) + got_ref_close(ref); if (commit) got_object_commit_close(commit); if (graph) ----------------------------------------------- commit b63a0a6de295765dcd906255f2d2d2bff35fc4f4 from: Omar Polo <op@omarpolo.com> date: Fri Sep 2 10:42:30 2022 UTC gotwebd: free in_repo_path in got_get_repo_commits diff 548422fdbb511de35fb644b9e8e5c6effbba76da b63a0a6de295765dcd906255f2d2d2bff35fc4f4 commit - 548422fdbb511de35fb644b9e8e5c6effbba76da commit + b63a0a6de295765dcd906255f2d2d2bff35fc4f4 blob - dcc24dae3dae23f69341a447788cdf594fc9a613 blob + a2964284b0746b56e407422c9347f61ca66b475d --- gotwebd/got_operations.c +++ gotwebd/got_operations.c @@ -550,6 +550,7 @@ done: if (graph) got_commit_graph_close(graph); got_ref_list_free(&refs); + free(in_repo_path); free(file_path); free(repo_path); free(id); ----------------------------------------------- commit 1089328056b8e8e6d9fb0a80575c437721ceb542 (main) from: Omar Polo <op@omarpolo.com> date: Fri Sep 2 10:42:39 2022 UTC gotwebd: plugs leaks in got_get_repo_commits call got_get_repo_commit only once and avoid leaking the field of repo_commit at each loop iteration. diff b63a0a6de295765dcd906255f2d2d2bff35fc4f4 1089328056b8e8e6d9fb0a80575c437721ceb542 commit - b63a0a6de295765dcd906255f2d2d2bff35fc4f4 commit + 1089328056b8e8e6d9fb0a80575c437721ceb542 blob - a2964284b0746b56e407422c9347f61ca66b475d blob + 5ea76c0eefd10ebe0ddf57279275a15c27edd525 --- gotwebd/got_operations.c +++ gotwebd/got_operations.c @@ -355,10 +355,6 @@ got_get_repo_commits(struct request *c, int limit) goto done; } - error = got_init_repo_commit(&repo_commit); - if (error) - goto done; - /* * XXX: jumping directly to a commit id via * got_repo_match_object_id_prefix significantly improves performance, @@ -439,7 +435,8 @@ got_get_repo_commits(struct request *c, int limit) for (;;) { if (limit_chk == ((limit * qs->page) - (limit - 1)) && - commit_found == 0 && repo_commit->commit_id != NULL) { + commit_found == 0 && repo_commit && + repo_commit->commit_id != NULL) { t->prev_id = strdup(repo_commit->commit_id); if (t->prev_id == NULL) { error = got_error_from_errno("strdup"); @@ -466,6 +463,12 @@ got_get_repo_commits(struct request *c, int limit) if (error) goto done; + error = got_init_repo_commit(&repo_commit); + if (error) + goto done; + + TAILQ_INSERT_TAIL(&t->repo_commits, repo_commit, entry); + error = got_get_repo_commit(c, repo_commit, commit, &refs, id); if (error) @@ -485,18 +488,6 @@ got_get_repo_commits(struct request *c, int limit) } } - struct repo_commit *new_repo_commit = NULL; - error = got_init_repo_commit(&new_repo_commit); - if (error) - goto done; - - TAILQ_INSERT_TAIL(&t->repo_commits, new_repo_commit, entry); - - error = got_get_repo_commit(c, new_repo_commit, commit, - &refs, id); - if (error) - goto done; - free(id); id = NULL; @@ -513,7 +504,7 @@ got_get_repo_commits(struct request *c, int limit) */ if (chk_next && (qs->action == BRIEFS || qs->action == COMMITS || qs->action == SUMMARY)) { - t->next_id = strdup(new_repo_commit->commit_id); + t->next_id = strdup(repo_commit->commit_id); if (t->next_id == NULL) { error = got_error_from_errno("strdup"); goto done; @@ -522,9 +513,9 @@ got_get_repo_commits(struct request *c, int limit) got_object_commit_close(commit); commit = NULL; } - TAILQ_REMOVE(&t->repo_commits, new_repo_commit, + TAILQ_REMOVE(&t->repo_commits, repo_commit, entry); - gotweb_free_repo_commit(new_repo_commit); + gotweb_free_repo_commit(repo_commit); goto done; } } @@ -542,7 +533,6 @@ got_get_repo_commits(struct request *c, int limit) } } done: - gotweb_free_repo_commit(repo_commit); if (ref) got_ref_close(ref); if (commit)
gotwebd: plug memleaks in got_get_repo_commits