"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Mark Jamsek <mark@jamsek.com>
Subject:
Re: plug some leaks in `got import`
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Thu, 12 Dec 2024 15:50:02 +1100

Download raw body.

Thread
Stefan Sperling <stsp@stsp.name> 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 <mark@jamsek.dev>
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 <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68