From: Mark Jamsek Subject: Re: plug some leaks in `got import` To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Thu, 12 Dec 2024 15:50:02 +1100 Stefan Sperling wrote: > On Wed, Dec 11, 2024 at 10:59:51PM +1100, Mark Jamsek wrote: > > Spotted by chance when investigating moving get_author() to the library: > > we leak some objects with early returns in a couple error cases instead > > of going to done to cleanup; and we also forgot to free the path_dir > > char pointer, and ignores pathlist. > > ok stsp@ Thanks, stsp. But I forgot all about cvg :/ Below diff is of the same fixes applied to cvg.c:cmd_import() commit 3b219c0407e428cff00f589dcfa8f3a572cbef20 from: Mark Jamsek date: Thu Dec 12 04:40:21 2024 UTC cvg: plug a couple leaks in `cvg import` Make sure we always cleanup in the error case; free the ignore pathlist; and free the path_dir char pointer (like e278a869d0). M cvg/cvg.c | 11+ 5- 1 file changed, 11 insertions(+), 5 deletions(-) commit - e278a869d0de428a352943747454613de10b132f commit + 3b219c0407e428cff00f589dcfa8f3a572cbef20 blob - 14f6f69962facd1f76c094458fd23bec1fabb9f1 blob + 1896ba4a7e44d840306970c2158e0421ed4b7903 --- cvg/cvg.c +++ cvg/cvg.c @@ -791,8 +791,10 @@ cmd_import(int argc, char *argv[]) if (repo_path == NULL) { repo_path = getcwd(NULL, 0); - if (repo_path == NULL) - return got_error_from_errno("getcwd"); + if (repo_path == NULL) { + error = got_error_from_errno("getcwd"); + goto done; + } } got_path_strip_trailing_slashes(repo_path); error = get_gitconfig_path(&gitconfig_path); @@ -807,15 +809,17 @@ cmd_import(int argc, char *argv[]) error = get_author(&author, repo, NULL); if (error) - return error; + goto done; /* * Don't let the user create a branch name with a leading '-'. * While technically a valid reference name, this case is usually * an unintended typo. */ - if (branch_name && branch_name[0] == '-') - return got_error_path(branch_name, GOT_ERR_REF_NAME_MINUS); + if (branch_name && branch_name[0] == '-') { + error = got_error_path(branch_name, GOT_ERR_REF_NAME_MINUS); + goto done; + } error = got_ref_open(&head_ref, repo, GOT_REF_HEAD, 0); if (error && error->code != GOT_ERR_NOT_REF) @@ -955,8 +959,10 @@ done: getprogname(), logmsg_path); } else if (logmsg_path && unlink(logmsg_path) == -1 && error == NULL) error = got_error_from_errno2("unlink", logmsg_path); + got_pathlist_free(&ignores, GOT_PATHLIST_FREE_NONE); free(logmsg); free(logmsg_path); + free(path_dir); free(repo_path); free(editor); free(new_commit_id); -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68