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

From:
Mark Jamsek <mark@jamsek.com>
Subject:
Re: refactor got_pathlist_free() to optionally free path and data pointers
To:
Game of Trees <gameoftrees@openbsd.org>
Date:
Tue, 10 Jan 2023 17:54:22 +1100

Download raw body.

Thread
On 23-01-09 03:53PM, Stefan Sperling wrote:
> On Tue, Jan 10, 2023 at 01:28:23AM +1100, Mark Jamsek wrote:
> > As briefly discussed with stsp, refactor got_pathlist_free() to
> > optionally free member pointers.
> > 
> > Although this touches a lot of code, the change is quite simple.
> > According to grep I haven't missed anything, but there's always
> > a non-zero chance I haven't got the translation right in every case!
> 
> I went through and did spot one conversion problem. See below.
> 

Thanks! The below diff is rebased on top of main. I've added your fixes
plus two more things:

  1. set path and data pointers to NULL in got_pathlist_free()
  2. added GOT_PATHLIST_FREE_ALL shortcut for
     GOT_PATHLIST_FREE_{data,path}

The first for a little double-free safety-net, and the second purely
because it saves some wrapping (and some typing in the future :), but
I can drop this if you prefer?

diffstat refs/heads/main refs/heads/pathlist
 M  got/got.c                                |  36+  97-
 M  gotadmin/gotadmin.c                      |   1+   1-
 M  include/got_path.h                       |   7+   1-
 M  include/got_worktree.h                   |   2+   5-
 M  lib/fetch.c                              |   3+  15-
 M  lib/fileindex.c                          |   2+  12-
 M  lib/path.c                               |   9+   1-
 M  lib/repository.c                         |   2+   5-
 M  lib/send.c                               |   2+   2-
 M  lib/worktree.c                           |   9+  28-
 M  libexec/got-fetch-pack/got-fetch-pack.c  |   4+  15-
 M  libexec/got-send-pack/got-send-pack.c    |   4+  17-
 M  regress/path/path_test.c                 |   2+   2-
 M  tog/tog.c                                |   1+   3-

14 files changed, 84 insertions(+), 204 deletions(-)

diff refs/heads/main refs/heads/pathlist
commit - a76e88e58fb716d5dded83442b153b60687283cb
commit + d2d9c6c64fe022cb175f9c1e14e9783c0fa02840
blob - 9c142650fd13686238b78aa2d167eaa7b856cffd
blob + 7f294d2ed40076ca1dedf56ff7581a9d161936e6
--- got/got.c
+++ got/got.c
@@ -1919,18 +1919,10 @@ done:
 		if (error == NULL)
 			error = close_err;
 	}
-	TAILQ_FOREACH(pe, &refs, entry) {
-		free((void *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&refs);
-	TAILQ_FOREACH(pe, &symrefs, entry) {
-		free((void *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&symrefs);
-	got_pathlist_free(&wanted_branches);
-	got_pathlist_free(&wanted_refs);
+	got_pathlist_free(&refs, GOT_PATHLIST_FREE_ALL);
+	got_pathlist_free(&symrefs, GOT_PATHLIST_FREE_ALL);
+	got_pathlist_free(&wanted_branches, GOT_PATHLIST_FREE_NONE);
+	got_pathlist_free(&wanted_refs, GOT_PATHLIST_FREE_NONE);
 	free(pack_hash);
 	free(proto);
 	free(host);
@@ -2735,18 +2727,10 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	TAILQ_FOREACH(pe, &refs, entry) {
-		free((void *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&refs);
-	TAILQ_FOREACH(pe, &symrefs, entry) {
-		free((void *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&symrefs);
-	got_pathlist_free(&wanted_branches);
-	got_pathlist_free(&wanted_refs);
+	got_pathlist_free(&refs, GOT_PATHLIST_FREE_ALL);
+	got_pathlist_free(&symrefs, GOT_PATHLIST_FREE_ALL);
+	got_pathlist_free(&wanted_branches, GOT_PATHLIST_FREE_NONE);
+	got_pathlist_free(&wanted_refs, GOT_PATHLIST_FREE_NONE);
 	free(id_str);
 	free(cwd);
 	free(repo_path);
@@ -3180,7 +3164,7 @@ done:
 		got_ref_close(head_ref);
 	if (ref)
 		got_ref_close(ref);
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_NONE);
 	free(commit_id_str);
 	free(commit_id);
 	free(repo_path);
@@ -3613,9 +3597,7 @@ done:
 			error = pack_err;
 	}
 	free(worktree_path);
-	TAILQ_FOREACH(pe, &paths, entry)
-		free((char *)pe->path);
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_PATH);
 	free(commit_id);
 	free(commit_id_str);
 	return error;
@@ -4309,7 +4291,6 @@ print_commits(struct got_object_id *root_id, struct go
 	struct got_object_qid *qid;
 	struct got_commit_object *commit;
 	struct got_pathlist_head changed_paths;
-	struct got_pathlist_entry *pe;
 
 	STAILQ_INIT(&reversed_commits);
 	TAILQ_INIT(&changed_paths);
@@ -4371,11 +4352,8 @@ print_commits(struct got_object_id *root_id, struct go
 			}
 			if (have_match == 0) {
 				got_object_commit_close(commit);
-				TAILQ_FOREACH(pe, &changed_paths, entry) {
-					free((char *)pe->path);
-					free(pe->data);
-				}
-				got_pathlist_free(&changed_paths);
+				got_pathlist_free(&changed_paths,
+				    GOT_PATHLIST_FREE_ALL);
 				continue;
 			}
 		}
@@ -4404,11 +4382,7 @@ print_commits(struct got_object_id *root_id, struct go
 		    (end_id && got_object_id_cmp(&id, end_id) == 0))
 			break;
 
-		TAILQ_FOREACH(pe, &changed_paths, entry) {
-			free((char *)pe->path);
-			free(pe->data);
-		}
-		got_pathlist_free(&changed_paths);
+		got_pathlist_free(&changed_paths, GOT_PATHLIST_FREE_ALL);
 	}
 	if (reverse_display_order) {
 		STAILQ_FOREACH(qid, &reversed_commits, entry) {
@@ -4437,11 +4411,7 @@ print_commits(struct got_object_id *root_id, struct go
 			got_object_commit_close(commit);
 			if (err)
 				break;
-			TAILQ_FOREACH(pe, &changed_paths, entry) {
-				free((char *)pe->path);
-				free(pe->data);
-			}
-			got_pathlist_free(&changed_paths);
+			got_pathlist_free(&changed_paths, GOT_PATHLIST_FREE_ALL);
 		}
 	}
 done:
@@ -4450,11 +4420,7 @@ done:
 		STAILQ_REMOVE_HEAD(&reversed_commits, entry);
 		got_object_qid_free(qid);
 	}
-	TAILQ_FOREACH(pe, &changed_paths, entry) {
-		free((char *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&changed_paths);
+	got_pathlist_free(&changed_paths, GOT_PATHLIST_FREE_ALL);
 	if (search_pattern)
 		regfree(&regex);
 	got_commit_graph_close(graph);
@@ -5051,7 +5017,6 @@ cmd_diff(int argc, char *argv[])
 	const char *errstr;
 	struct got_reflist_head refs;
 	struct got_pathlist_head diffstat_paths, paths;
-	struct got_pathlist_entry *pe;
 	FILE *f1 = NULL, *f2 = NULL, *outfile = NULL;
 	int fd1 = -1, fd2 = -1;
 	int *pack_fds = NULL;
@@ -5481,14 +5446,8 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	TAILQ_FOREACH(pe, &paths, entry)
-		free((char *)pe->path);
-	got_pathlist_free(&paths);
-	TAILQ_FOREACH(pe, &diffstat_paths, entry) {
-		free((char *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&diffstat_paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_PATH);
+	got_pathlist_free(&diffstat_paths, GOT_PATHLIST_FREE_ALL);
 	got_ref_list_free(&refs);
 	if (outfile && fclose(outfile) == EOF && error == NULL)
 		error = got_error_from_errno("fclose");
@@ -6260,7 +6219,6 @@ cmd_status(int argc, char *argv[])
 	struct got_status_arg st;
 	char *cwd = NULL;
 	struct got_pathlist_head paths;
-	struct got_pathlist_entry *pe;
 	int ch, i, no_ignores = 0;
 	int *pack_fds = NULL;
 
@@ -6357,9 +6315,7 @@ done:
 			error = pack_err;
 	}
 
-	TAILQ_FOREACH(pe, &paths, entry)
-		free((char *)pe->path);
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_PATH);
 	free(cwd);
 	return error;
 }
@@ -6893,7 +6849,6 @@ cmd_branch(int argc, char *argv[])
 	const char *delref = NULL, *commit_id_arg = NULL;
 	struct got_reference *ref = NULL;
 	struct got_pathlist_head paths;
-	struct got_pathlist_entry *pe;
 	struct got_object_id *commit_id = NULL;
 	char *commit_id_str = NULL;
 	int *pack_fds = NULL;
@@ -7092,9 +7047,7 @@ done:
 	free(repo_path);
 	free(commit_id);
 	free(commit_id_str);
-	TAILQ_FOREACH(pe, &paths, entry)
-		free((char *)pe->path);
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_PATH);
 	return error;
 }
 
@@ -7887,9 +7840,7 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	TAILQ_FOREACH(pe, &paths, entry)
-		free((char *)pe->path);
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_PATH);
 	free(cwd);
 	return error;
 }
@@ -8054,9 +8005,7 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	TAILQ_FOREACH(pe, &paths, entry)
-		free((char *)pe->path);
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_PATH);
 	free(cwd);
 	return error;
 }
@@ -9461,14 +9410,12 @@ done:
 	}
 	if (ref)
 		got_ref_close(ref);
-	got_pathlist_free(&branches);
-	got_pathlist_free(&tags);
+	got_pathlist_free(&branches, GOT_PATHLIST_FREE_NONE);
+	got_pathlist_free(&tags, GOT_PATHLIST_FREE_NONE);
 	got_ref_list_free(&all_branches);
 	got_ref_list_free(&all_tags);
-	got_pathlist_free(&delete_args);
-	TAILQ_FOREACH(pe, &delete_branches, entry)
-		free((char *)pe->path);
-	got_pathlist_free(&delete_branches);
+	got_pathlist_free(&delete_args, GOT_PATHLIST_FREE_NONE);
+	got_pathlist_free(&delete_branches, GOT_PATHLIST_FREE_PATH);
 	free(cwd);
 	free(repo_path);
 	free(proto);
@@ -10531,7 +10478,7 @@ cmd_rebase(int argc, char *argv[])
 			error = got_worktree_checkout_files(worktree,
 			    &paths, repo, update_progress, &upa,
 			    check_cancelled, NULL);
-			got_pathlist_free(&paths);
+			got_pathlist_free(&paths, GOT_PATHLIST_FREE_NONE);
 			if (error)
 				goto done;
 			if (upa.did_something) {
@@ -10629,13 +10576,13 @@ cmd_rebase(int argc, char *argv[])
 				if (error)
 					goto done;
 			}
-			got_worktree_rebase_pathlist_free(&merged_paths);
+			got_pathlist_free(&merged_paths, GOT_PATHLIST_FREE_PATH);
 			break;
 		}
 
 		error = rebase_commit(&merged_paths, worktree, fileindex,
 		    tmp_branch, committer, commit_id, repo);
-		got_worktree_rebase_pathlist_free(&merged_paths);
+		got_pathlist_free(&merged_paths, GOT_PATHLIST_FREE_PATH);
 		if (error)
 			goto done;
 	}
@@ -11952,7 +11899,8 @@ cmd_histedit(int argc, char *argv[])
 				error = got_worktree_status(worktree, &paths,
 				    repo, 0, check_local_changes, &have_changes,
 				    check_cancelled, NULL);
-				got_pathlist_free(&paths);
+				got_pathlist_free(&paths,
+				    GOT_PATHLIST_FREE_NONE);
 				if (error) {
 					if (error->code != GOT_ERR_CANCELLED)
 						goto done;
@@ -12012,7 +11960,7 @@ cmd_histedit(int argc, char *argv[])
 				if (error)
 					goto done;
 			}
-			got_worktree_rebase_pathlist_free(&merged_paths);
+			got_pathlist_free(&merged_paths, GOT_PATHLIST_FREE_PATH);
 			break;
 		}
 
@@ -12024,7 +11972,7 @@ cmd_histedit(int argc, char *argv[])
 			printf("Stopping histedit for amending commit %s\n",
 			    id_str);
 			free(id_str);
-			got_worktree_rebase_pathlist_free(&merged_paths);
+			got_pathlist_free(&merged_paths, GOT_PATHLIST_FREE_PATH);
 			error = got_worktree_histedit_postpone(worktree,
 			    fileindex);
 			goto done;
@@ -12039,7 +11987,7 @@ cmd_histedit(int argc, char *argv[])
 
 		error = histedit_commit(&merged_paths, worktree, fileindex,
 		    tmp_branch, hle, committer, repo);
-		got_worktree_rebase_pathlist_free(&merged_paths);
+		got_pathlist_free(&merged_paths, GOT_PATHLIST_FREE_PATH);
 		if (error)
 			goto done;
 	}
@@ -12585,7 +12533,6 @@ cmd_stage(int argc, char *argv[])
 	struct got_worktree *worktree = NULL;
 	char *cwd = NULL;
 	struct got_pathlist_head paths;
-	struct got_pathlist_entry *pe;
 	int ch, list_stage = 0, pflag = 0, allow_bad_symlinks = 0;
 	FILE *patch_script_file = NULL;
 	const char *patch_script_path = NULL;
@@ -12698,9 +12645,7 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	TAILQ_FOREACH(pe, &paths, entry)
-		free((char *)pe->path);
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_PATH);
 	free(cwd);
 	return error;
 }
@@ -12722,7 +12667,6 @@ cmd_unstage(int argc, char *argv[])
 	struct got_worktree *worktree = NULL;
 	char *cwd = NULL;
 	struct got_pathlist_head paths;
-	struct got_pathlist_entry *pe;
 	int ch, pflag = 0;
 	struct got_update_progress_arg upa;
 	FILE *patch_script_file = NULL;
@@ -12821,9 +12765,7 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	TAILQ_FOREACH(pe, &paths, entry)
-		free((char *)pe->path);
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_PATH);
 	free(cwd);
 	return error;
 }
@@ -13264,7 +13206,6 @@ cmd_info(int argc, char *argv[])
 	struct got_worktree *worktree = NULL;
 	char *cwd = NULL, *id_str = NULL;
 	struct got_pathlist_head paths;
-	struct got_pathlist_entry *pe;
 	char *uuidstr = NULL;
 	int ch, show_files = 0;
 
@@ -13363,9 +13304,7 @@ done:
 done:
 	if (worktree)
 		got_worktree_close(worktree);
-	TAILQ_FOREACH(pe, &paths, entry)
-		free((char *)pe->path);
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_PATH);
 	free(cwd);
 	free(id_str);
 	free(uuidstr);
blob - aca013eba6083c114897f57932c5b1565d61bf14
blob + b36b50b4e34e52be3dca38a6cc051e5fab489e0c
--- gotadmin/gotadmin.c
+++ gotadmin/gotadmin.c
@@ -827,7 +827,7 @@ done:
 		if (error == NULL)
 			error = pack_err;
 	}
-	got_pathlist_free(&exclude_args);
+	got_pathlist_free(&exclude_args, GOT_PATHLIST_FREE_NONE);
 	got_ref_list_free(&exclude_refs);
 	got_ref_list_free(&include_refs);
 	free(id_str);
blob - 7cb114b362ef4f4a6d3f385fb37066d5a97691bd
blob + aa121bb3af4c0cba66fb0d062f2f6a8cc33fdd27
--- include/got_path.h
+++ include/got_path.h
@@ -92,8 +92,14 @@ const struct got_error *got_pathlist_append(struct got
 const struct got_error *got_pathlist_append(struct got_pathlist_head *,
     const char *, void *);
 
+/* Flags passed to got_pathlist_free() to control which pointers are freed. */
+#define GOT_PATHLIST_FREE_NONE	0	  /* pathlist entry only */
+#define GOT_PATHLIST_FREE_PATH	(1 << 0)  /* entry and path pointer */
+#define GOT_PATHLIST_FREE_DATA	(1 << 1)  /* entry and data pointer */
+#define GOT_PATHLIST_FREE_ALL	(GOT_PATHLIST_FREE_PATH|GOT_PATHLIST_FREE_DATA)
+
 /* Free resources allocated for a path list. */
-void got_pathlist_free(struct got_pathlist_head *);
+void got_pathlist_free(struct got_pathlist_head *, int);
 
 /* Attempt to create a directory at a given path. */
 const struct got_error *got_path_mkdir(const char *);
blob - 3d11d1ed4657d8fa1f4809c2b04fe5b891fea563
blob + d3c26b9a044948f5b93e8f2f6adf673b8dd7e599
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -287,7 +287,7 @@ const struct got_error *got_worktree_rebase_in_progres
  * progress callback. Also populate a list of affected paths which should
  * be passed to got_worktree_rebase_commit() after a conflict-free merge.
  * This list must be initialized with TAILQ_INIT() and disposed of with
- * got_worktree_rebase_pathlist_free().
+ * got_pathlist_free(list, GOT_PATHLIST_FREE_PATH).
  */
 const struct got_error *got_worktree_rebase_merge_files(
     struct got_pathlist_head *, struct got_worktree *, struct got_fileindex *,
@@ -305,9 +305,6 @@ const struct got_error *got_worktree_rebase_commit(str
     struct got_reference *, const char *, struct got_commit_object *,
     struct got_object_id *, struct got_repository *);
 
-/* Free a list of merged paths from got_worktree_merge_files. */
-void got_worktree_rebase_pathlist_free(struct got_pathlist_head *);
-
 /* Postpone the rebase operation. Should be called after a merge conflict. */
 const struct got_error *got_worktree_rebase_postpone(struct got_worktree *,
     struct got_fileindex *);
@@ -362,7 +359,7 @@ const struct got_error *got_worktree_histedit_in_progr
  * progress callback. Also populate a list of affected paths which should
  * be passed to got_worktree_histedit_commit() after a conflict-free merge.
  * This list must be initialized with TAILQ_INIT() and disposed of with
- * got_worktree_rebase_pathlist_free().
+ * got_pathlist_free(list, GOT_PATHLIST_FREE_PATH).
  */
 const struct got_error *got_worktree_histedit_merge_files(
     struct got_pathlist_head *, struct got_worktree *, struct got_fileindex *,
blob - 0dd641076040553ff06ef8320ae4686dabdd6786
blob + a006844341e6b3ee524360e3929fe3eee0536090
--- lib/fetch.c
+++ lib/fetch.c
@@ -540,25 +540,13 @@ done:
 	free(packpath);
 	free(progress);
 
-	TAILQ_FOREACH(pe, &have_refs, entry) {
-		free((char *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&have_refs);
+	got_pathlist_free(&have_refs, GOT_PATHLIST_FREE_ALL);
 	got_ref_list_free(&my_refs);
 	if (err) {
 		free(*pack_hash);
 		*pack_hash = NULL;
-		TAILQ_FOREACH(pe, refs, entry) {
-			free((void *)pe->path);
-			free(pe->data);
-		}
-		got_pathlist_free(refs);
-		TAILQ_FOREACH(pe, symrefs, entry) {
-			free((void *)pe->path);
-			free(pe->data);
-		}
-		got_pathlist_free(symrefs);
+		got_pathlist_free(refs, GOT_PATHLIST_FREE_ALL);
+		got_pathlist_free(symrefs, GOT_PATHLIST_FREE_ALL);
 	}
 	return err;
 }
blob - fca0ead4f2705e742e6d52e30f094a01b9369853
blob + 0762e831406c5e0e9f9e6eae03824defa43b6b00
--- lib/fileindex.c
+++ lib/fileindex.c
@@ -980,16 +980,6 @@ static void
 	return err;
 }
 
-static void
-free_dirlist(struct got_pathlist_head *dirlist)
-{
-	struct got_pathlist_entry *dle;
-
-	TAILQ_FOREACH(dle, dirlist, entry)
-		free(dle->data);
-	got_pathlist_free(dirlist);
-}
-
 static int
 have_tracked_file_in_dir(struct got_fileindex *fileindex, const char *path)
 {
@@ -1078,7 +1068,7 @@ walk_dir(struct got_pathlist_entry **next, struct got_
 			err = got_error_from_errno2("closedir", subdirpath);
 		free(subpath);
 		free(subdirpath);
-		free_dirlist(&subdirlist);
+		got_pathlist_free(&subdirlist, GOT_PATHLIST_FREE_DATA);
 		if (err)
 			return err;
 	}
@@ -1237,7 +1227,7 @@ got_fileindex_diff_dir(struct got_fileindex *fileindex
 
 	if (closedir(dir) == -1 && err == NULL)
 		err = got_error_from_errno2("closedir", path);
-	free_dirlist(&dirlist);
+	got_pathlist_free(&dirlist, GOT_PATHLIST_FREE_DATA);
 	return err;
 }
 
blob - 7e7eb38cbafe546d0c4a2210a07ac681d66462f3
blob + a43c93bf01b5cb12693933595ad320101db7fc5b
--- lib/path.c
+++ lib/path.c
@@ -272,11 +272,19 @@ got_pathlist_free(struct got_pathlist_head *pathlist)
 }
 
 void
-got_pathlist_free(struct got_pathlist_head *pathlist)
+got_pathlist_free(struct got_pathlist_head *pathlist, int freemask)
 {
 	struct got_pathlist_entry *pe;
 
 	while ((pe = TAILQ_FIRST(pathlist)) != NULL) {
+		if (freemask & GOT_PATHLIST_FREE_PATH) {
+			free((char *)pe->path);
+			pe->path = NULL;
+		}
+		if (freemask & GOT_PATHLIST_FREE_DATA) {
+			free(pe->data);
+			pe->data = NULL;
+		}
 		TAILQ_REMOVE(pathlist, pe, entry);
 		free(pe);
 	}
blob - 7500a424c3c703e986507e47bad61b771851f850
blob + e754fef75fcc2d66013a8f85b64547acc0aea1c4
--- lib/repository.c
+++ lib/repository.c
@@ -790,7 +790,6 @@ got_repo_close(struct got_repository *repo)
 {
 	const struct got_error *err = NULL, *child_err;
 	struct got_packidx_bloom_filter *bf;
-	struct got_pathlist_entry *pe;
 	size_t i;
 
 	for (i = 0; i < repo->pack_cache_size; i++) {
@@ -852,9 +851,7 @@ got_repo_close(struct got_repository *repo)
 		free(repo->extensions[i]);
 	free(repo->extensions);
 
-	TAILQ_FOREACH(pe, &repo->packidx_paths, entry)
-		free((void *)pe->path);
-	got_pathlist_free(&repo->packidx_paths);
+	got_pathlist_free(&repo->packidx_paths, GOT_PATHLIST_FREE_PATH);
 	free(repo);
 
 	return err;
@@ -2175,7 +2172,7 @@ done:
 done:
 	if (dir)
 		closedir(dir);
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_NONE);
 	return err;
 }
 
blob - ade0d5069d9f6bac709c4e0f2ea1dac629315b46
blob + ec59732b5d47bf06ab52a0c1cced9366c7816e51
--- lib/send.c
+++ lib/send.c
@@ -730,8 +730,8 @@ done:
 		err = got_error_from_errno("close");
 
 	got_ref_list_free(&refs);
-	got_pathlist_free(&have_refs);
-	got_pathlist_free(&their_refs);
+	got_pathlist_free(&have_refs, GOT_PATHLIST_FREE_NONE);
+	got_pathlist_free(&their_refs, GOT_PATHLIST_FREE_NONE);
 	for (i = 0; i < nours; i++)
 		free(our_ids[i]);
 	free(our_ids);
blob - e92fe5aa6a9932cfb59504fd20073ebc1fb4f6d4
blob + 76e39597d9c660a39120119d1f5538a0f5baf8ce
--- lib/worktree.c
+++ lib/worktree.c
@@ -3411,26 +3411,16 @@ free_ignorelist(struct got_pathlist_head *ignorelist)
 }
 
 static void
-free_ignorelist(struct got_pathlist_head *ignorelist)
-{
-	struct got_pathlist_entry *pe;
-
-	TAILQ_FOREACH(pe, ignorelist, entry)
-		free((char *)pe->path);
-	got_pathlist_free(ignorelist);
-}
-
-static void
 free_ignores(struct got_pathlist_head *ignores)
 {
 	struct got_pathlist_entry *pe;
 
 	TAILQ_FOREACH(pe, ignores, entry) {
 		struct got_pathlist_head *ignorelist = pe->data;
-		free_ignorelist(ignorelist);
-		free((char *)pe->path);
+
+		got_pathlist_free(ignorelist, GOT_PATHLIST_FREE_PATH);
 	}
-	got_pathlist_free(ignores);
+	got_pathlist_free(ignores, GOT_PATHLIST_FREE_PATH);
 }
 
 static const struct got_error *
@@ -3484,7 +3474,7 @@ done:
 	free(line);
 	if (err || pe == NULL) {
 		free(dirpath);
-		free_ignorelist(ignorelist);
+		got_pathlist_free(ignorelist, GOT_PATHLIST_FREE_PATH);
 	}
 	return err;
 }
@@ -5782,7 +5772,7 @@ done:
 	/* Write new list of entries; deleted entries have been dropped. */
 	err = got_object_tree_create(new_tree_id, &paths, *nentries, repo);
 done:
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_NONE);
 	return err;
 }
 
@@ -6242,9 +6232,10 @@ done:
 		err = unlockerr;
 	TAILQ_FOREACH(pe, &commitable_paths, entry) {
 		struct got_commitable *ct = pe->data;
+
 		free_commitable(ct);
 	}
-	got_pathlist_free(&commitable_paths);
+	got_pathlist_free(&commitable_paths, GOT_PATHLIST_FREE_NONE);
 	if (diff_path && unlink(diff_path) == -1 && err == NULL)
 		err = got_error_from_errno2("unlink", diff_path);
 	free(diff_path);
@@ -6593,17 +6584,6 @@ void
 	return err;
 }
 
-void
-got_worktree_rebase_pathlist_free(struct got_pathlist_head *merged_paths)
-{
-	struct got_pathlist_entry *pe;
-
-	TAILQ_FOREACH(pe, merged_paths, entry)
-		free((char *)pe->path);
-
-	got_pathlist_free(merged_paths);
-}
-
 static const struct got_error *
 store_commit_id(const char *commit_ref_name, struct got_object_id *commit_id,
     int is_rebase, struct got_repository *repo)
@@ -7957,9 +7937,10 @@ done:
 done:
 	TAILQ_FOREACH(pe, &commitable_paths, entry) {
 		struct got_commitable *ct = pe->data;
+
 		free_commitable(ct);
 	}
-	got_pathlist_free(&commitable_paths);
+	got_pathlist_free(&commitable_paths, GOT_PATHLIST_FREE_NONE);
 	free(fileindex_path);
 	return err;
 }
blob - 07e2042c17a6d66588622b9d6a7fb1e9c158cfbd
blob + cbcfef7d6f9e5c10b699ada020cdf69f79490487
--- libexec/got-fetch-pack/got-fetch-pack.c
+++ libexec/got-fetch-pack/got-fetch-pack.c
@@ -759,11 +759,7 @@ done:
 		    "pack file checksum mismatch");
 	}
 done:
-	TAILQ_FOREACH(pe, &symrefs, entry) {
-		free((void *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&symrefs);
+	got_pathlist_free(&symrefs, GOT_PATHLIST_FREE_ALL);
 	free(have);
 	free(want);
 	free(id_str);
@@ -777,14 +773,13 @@ main(int argc, char **argv)
 main(int argc, char **argv)
 {
 	const struct got_error *err = NULL;
-	int fetchfd, packfd = -1;
+	int fetchfd = -1, packfd = -1;
 	uint8_t pack_sha1[SHA1_DIGEST_LENGTH];
 	struct imsgbuf ibuf;
 	struct imsg imsg;
 	struct got_pathlist_head have_refs;
 	struct got_pathlist_head wanted_branches;
 	struct got_pathlist_head wanted_refs;
-	struct got_pathlist_entry *pe;
 	struct got_imsg_fetch_request fetch_req;
 	struct got_imsg_fetch_have_ref href;
 	struct got_imsg_fetch_wanted_branch wbranch;
@@ -990,14 +985,8 @@ done:
 	    fetch_req.fetch_all_branches, &wanted_branches,
 	    &wanted_refs, fetch_req.list_refs_only, &ibuf);
 done:
-	TAILQ_FOREACH(pe, &have_refs, entry) {
-		free((char *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&have_refs);
-	TAILQ_FOREACH(pe, &wanted_branches, entry)
-		free((char *)pe->path);
-	got_pathlist_free(&wanted_branches);
+	got_pathlist_free(&have_refs, GOT_PATHLIST_FREE_ALL);
+	got_pathlist_free(&wanted_branches, GOT_PATHLIST_FREE_PATH);
 	if (fetchfd != -1 && close(fetchfd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	if (packfd != -1 && close(packfd) == -1 && err == NULL)
blob - 1d25734c98460f62110adce7185d5a73c017c8ad
blob + 40b31c00d9ba7fd602c0df52657c1340c932af57
--- libexec/got-send-pack/got-send-pack.c
+++ libexec/got-send-pack/got-send-pack.c
@@ -593,11 +593,7 @@ done:
 
 	err = send_done(ibuf);
 done:
-	TAILQ_FOREACH(pe, &their_refs, entry) {
-		free((void *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&their_refs);
+	got_pathlist_free(&their_refs, GOT_PATHLIST_FREE_ALL);
 	free(id_str);
 	free(id);
 	free(refname);
@@ -609,12 +605,11 @@ main(int argc, char **argv)
 main(int argc, char **argv)
 {
 	const struct got_error *err = NULL;
-	int sendfd;
+	int sendfd = -1;
 	struct imsgbuf ibuf;
 	struct imsg imsg;
 	struct got_pathlist_head refs;
 	struct got_pathlist_head delete_refs;
-	struct got_pathlist_entry *pe;
 	struct got_imsg_send_request send_req;
 	struct got_imsg_send_ref href;
 	size_t datalen, i;
@@ -725,16 +720,8 @@ done:
 
 	err = send_pack(sendfd, &refs, &delete_refs, &ibuf);
 done:
-	TAILQ_FOREACH(pe, &refs, entry) {
-		free((char *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&refs);
-	TAILQ_FOREACH(pe, &delete_refs, entry) {
-		free((char *)pe->path);
-		free(pe->data);
-	}
-	got_pathlist_free(&delete_refs);
+	got_pathlist_free(&refs, GOT_PATHLIST_FREE_ALL);
+	got_pathlist_free(&delete_refs, GOT_PATHLIST_FREE_ALL);
 	if (sendfd != -1 && close(sendfd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	if (err != NULL && err->code != GOT_ERR_CANCELLED)  {
blob - 1b4e2a08ae74cbaca233fe92241caa8848454f5d
blob + fad15574f54715f8f62105579fbc58c1ca5d1bf1
--- regress/path/path_test.c
+++ regress/path/path_test.c
@@ -165,7 +165,7 @@ path_list(void)
 		i++;
 	}
 
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_NONE);
 	return 1;
 }
 
@@ -202,7 +202,7 @@ path_list_reverse_input(void)
 		i++;
 	}
 
-	got_pathlist_free(&paths);
+	got_pathlist_free(&paths, GOT_PATHLIST_FREE_NONE);
 	return 1;
 }
 
blob - 145254f50168ae87993a5b803db3c57555873c4b
blob + a8e19fd0434fff4255f3fda198af845263ea9816
--- tog/tog.c
+++ tog/tog.c
@@ -4711,8 +4711,6 @@ write_commit_info(struct got_diff_line **lines, size_t
 		    GOT_DIFF_LINE_CHANGES);
 		if (err)
 			goto done;
-		free((char *)pe->path);
-		free(pe->data);
 	}
 
 	fputc('\n', outfile);
@@ -4737,7 +4735,7 @@ done:
 	outoff++;
 	err = add_line_metadata(lines, nlines, outoff, GOT_DIFF_LINE_NONE);
 done:
-	got_pathlist_free(&changed_paths);
+	got_pathlist_free(&changed_paths, GOT_PATHLIST_FREE_ALL);
 	free(id_str);
 	free(logmsg);
 	free(refs_str);

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68