refactor got_pathlist_free() to optionally free path and data pointers

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! This also enabled removing a few bespoke list deallocation routines: got_worktree_rebase_pathlist_free() free_dirlist() free_ignorelist() There're two diffs below. The topmost initialises two fildes in got-{fetch,send}-pack libexec helpers. This is necessary because the refactor appears to have uncovered a sometimes unitialised error whenever we jump to done before assigning these variables. The second diff is the refactor, but you'll need to apply both to get a build. Regress is still passing which is promising too :) ----------------------------------------------- commit 18de6ad7fcc89bb0ada6fe5079f86d5b0d9dacc6 (main) from: Mark Jamsek <mark@jamsek.dev> date: Mon Jan 9 14:16:46 2023 UTC fix uninitialised fildes uncovered by refactor M libexec/got-fetch-pack/got-fetch-pack.c | 1+ 1- M libexec/got-send-pack/got-send-pack.c | 1+ 1- 2 files changed, 2 insertions(+), 2 deletions(-) diff 0f1e91bd00171818d19a4732c1d29f7a72ff9d07 18de6ad7fcc89bb0ada6fe5079f86d5b0d9dacc6 commit - 0f1e91bd00171818d19a4732c1d29f7a72ff9d07 commit + 18de6ad7fcc89bb0ada6fe5079f86d5b0d9dacc6 blob - 19f521a5dbdaea78f1198ec2c278679c0db28dfb blob + 089401a12806cab32526367726cbe8f7c19e4a07 --- libexec/got-fetch-pack/got-fetch-pack.c +++ libexec/got-fetch-pack/got-fetch-pack.c @@ -773,7 +773,7 @@ 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; blob - 10d9e4239a69135e45b60ad02da2564535e8d8db blob + 933d88aad619c42417b3a24210a4d595ccb97163 --- libexec/got-send-pack/got-send-pack.c +++ libexec/got-send-pack/got-send-pack.c @@ -605,7 +605,7 @@ 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; ----------------------------------------------- commit 0f1e91bd00171818d19a4732c1d29f7a72ff9d07 from: Mark Jamsek <mark@jamsek.dev> date: Mon Jan 9 14:11:18 2023 UTC refactor got_pathlist_free() to optionally free pointers M got/got.c | 37+ 92- M gotadmin/gotadmin.c | 1+ 1- M include/got_path.h | 4+ 1- M include/got_worktree.h | 2+ 5- M lib/fetch.c | 4+ 15- M lib/fileindex.c | 2+ 12- M lib/path.c | 5+ 1- M lib/repository.c | 2+ 5- M lib/send.c | 2+ 2- M lib/worktree.c | 7+ 36- M libexec/got-fetch-pack/got-fetch-pack.c | 3+ 14- M libexec/got-send-pack/got-send-pack.c | 3+ 16- M regress/path/path_test.c | 2+ 2- M tog/tog.c | 2+ 3- 14 files changed, 76 insertions(+), 205 deletions(-) diff 0b3f028dffa4ecc7aa72dc9132d53e9d056cc36f 0f1e91bd00171818d19a4732c1d29f7a72ff9d07 commit - 0b3f028dffa4ecc7aa72dc9132d53e9d056cc36f commit + 0f1e91bd00171818d19a4732c1d29f7a72ff9d07 blob - 9a89d5b62f1635fb4b385534abdd217ee4a06611 blob + d899043b8707dbe52a0732f74b0e23c300e66558 --- 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, PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); + got_pathlist_free(&symrefs, PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); + got_pathlist_free(&wanted_branches, PATHLIST_FREE_NONE); + got_pathlist_free(&wanted_refs, 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, PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); + got_pathlist_free(&symrefs, PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); + got_pathlist_free(&wanted_branches, PATHLIST_FREE_NONE); + got_pathlist_free(&wanted_refs, 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, 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, PATHLIST_FREE_PATH); free(commit_id); free(commit_id_str); return error; @@ -4294,7 +4276,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); @@ -4356,11 +4337,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, + PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); continue; } } @@ -4389,11 +4367,8 @@ 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, + PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); } if (reverse_display_order) { STAILQ_FOREACH(qid, &reversed_commits, entry) { @@ -4422,11 +4397,8 @@ 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, + PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); } } done: @@ -4435,11 +4407,8 @@ 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, + PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); if (search_pattern) regfree(&regex); got_commit_graph_close(graph); @@ -4997,7 +4966,6 @@ cmd_diff(int argc, char *argv[]) const char *errstr; struct got_reflist_head refs; struct got_pathlist_head paths; - struct got_pathlist_entry *pe; FILE *f1 = NULL, *f2 = NULL; int fd1 = -1, fd2 = -1; int *pack_fds = NULL; @@ -5365,9 +5333,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, PATHLIST_FREE_PATH); got_ref_list_free(&refs); if (f1 && fclose(f1) == EOF && error == NULL) error = got_error_from_errno("fclose"); @@ -6137,7 +6103,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; @@ -6234,9 +6199,7 @@ done: error = pack_err; } - TAILQ_FOREACH(pe, &paths, entry) - free((char *)pe->path); - got_pathlist_free(&paths); + got_pathlist_free(&paths, PATHLIST_FREE_PATH); free(cwd); return error; } @@ -6770,7 +6733,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; @@ -6969,9 +6931,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, PATHLIST_FREE_PATH); return error; } @@ -7764,9 +7724,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, PATHLIST_FREE_PATH); free(cwd); return error; } @@ -7931,9 +7889,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, PATHLIST_FREE_PATH); free(cwd); return error; } @@ -9338,14 +9294,12 @@ done: } if (ref) got_ref_close(ref); - got_pathlist_free(&branches); - got_pathlist_free(&tags); + got_pathlist_free(&branches, PATHLIST_FREE_NONE); + got_pathlist_free(&tags, 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, PATHLIST_FREE_NONE); + got_pathlist_free(&delete_branches, PATHLIST_FREE_PATH); free(cwd); free(repo_path); free(proto); @@ -10408,7 +10362,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, PATHLIST_FREE_NONE); if (error) goto done; if (upa.did_something) { @@ -10506,13 +10460,13 @@ cmd_rebase(int argc, char *argv[]) if (error) goto done; } - got_worktree_rebase_pathlist_free(&merged_paths); + got_pathlist_free(&merged_paths, 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, PATHLIST_FREE_PATH); if (error) goto done; } @@ -11829,7 +11783,7 @@ 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, PATHLIST_FREE_NONE); if (error) { if (error->code != GOT_ERR_CANCELLED) goto done; @@ -11889,7 +11843,7 @@ cmd_histedit(int argc, char *argv[]) if (error) goto done; } - got_worktree_rebase_pathlist_free(&merged_paths); + got_pathlist_free(&merged_paths, PATHLIST_FREE_PATH); break; } @@ -11901,7 +11855,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, PATHLIST_FREE_PATH); error = got_worktree_histedit_postpone(worktree, fileindex); goto done; @@ -11916,7 +11870,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, PATHLIST_FREE_PATH); if (error) goto done; } @@ -12462,7 +12416,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; @@ -12575,9 +12528,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, PATHLIST_FREE_PATH); free(cwd); return error; } @@ -12599,7 +12550,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; @@ -12698,9 +12648,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, PATHLIST_FREE_PATH); free(cwd); return error; } @@ -13141,7 +13089,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; @@ -13240,9 +13187,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, PATHLIST_FREE_PATH); free(cwd); free(id_str); free(uuidstr); blob - aca013eba6083c114897f57932c5b1565d61bf14 blob + c9061dff53f312bd6e6aedd2a8de8f42e4733365 --- 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, PATHLIST_FREE_NONE); got_ref_list_free(&exclude_refs); got_ref_list_free(&include_refs); free(id_str); blob - 7cb114b362ef4f4a6d3f385fb37066d5a97691bd blob + 9eb5ed8033a8621005debb45847be7cb1fb7598a --- include/got_path.h +++ include/got_path.h @@ -93,7 +93,10 @@ void got_pathlist_free(struct got_pathlist_head *); const char *, void *); /* Free resources allocated for a path list. */ -void got_pathlist_free(struct got_pathlist_head *); +#define PATHLIST_FREE_NONE 0 +#define PATHLIST_FREE_PATH 1 << 1 +#define PATHLIST_FREE_DATA 1 << 2 +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 + 03251e8453d5d2cda24c3e28f325f838e912dec0 --- 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, 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, 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 + 0323b29cb34ded43aacdc4c2d63c1767892963d6 --- lib/fetch.c +++ lib/fetch.c @@ -540,25 +540,14 @@ 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, PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); 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, PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); + got_pathlist_free(symrefs, + PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); } return err; } blob - fca0ead4f2705e742e6d52e30f094a01b9369853 blob + 3755c697368b71e6869edf43d0e8f814edca1646 --- 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, 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, PATHLIST_FREE_DATA); return err; } blob - 7e7eb38cbafe546d0c4a2210a07ac681d66462f3 blob + 254d4d6ac0903e2931cb5eef3b3f51969324f20c --- lib/path.c +++ lib/path.c @@ -272,11 +272,15 @@ 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 & PATHLIST_FREE_PATH) + free((char *)pe->path); + if (freemask & PATHLIST_FREE_DATA) + free(pe->data); TAILQ_REMOVE(pathlist, pe, entry); free(pe); } blob - 7500a424c3c703e986507e47bad61b771851f850 blob + e54cc3da8f00840b321f8accf3aa5674b0316bd1 --- 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, 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, PATHLIST_FREE_NONE); return err; } blob - ade0d5069d9f6bac709c4e0f2ea1dac629315b46 blob + a6d3de33681964a7690e4c303d9a84225bbc2845 --- 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, PATHLIST_FREE_NONE); + got_pathlist_free(&their_refs, PATHLIST_FREE_NONE); for (i = 0; i < nours; i++) free(our_ids[i]); free(our_ids); blob - b40f67cf87889e4cd271c0870db2a357160443ad blob + cb0a0ce6a8cc335393e2e827663b61d0f7206e38 --- 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, PATHLIST_FREE_PATH); } - got_pathlist_free(ignores); + got_pathlist_free(ignores, 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, 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, PATHLIST_FREE_NONE); return err; } @@ -6240,11 +6230,7 @@ done: unlockerr = lock_worktree(worktree, LOCK_SH); if (unlockerr && err == NULL) 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, PATHLIST_FREE_DATA); if (diff_path && unlink(diff_path) == -1 && err == NULL) err = got_error_from_errno2("unlink", diff_path); free(diff_path); @@ -6593,17 +6579,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) @@ -7955,11 +7930,7 @@ done: if (sync_err && err == NULL) err = sync_err; 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, PATHLIST_FREE_DATA); free(fileindex_path); return err; } blob - 07e2042c17a6d66588622b9d6a7fb1e9c158cfbd blob + 19f521a5dbdaea78f1198ec2c278679c0db28dfb --- 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, PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); free(have); free(want); free(id_str); @@ -784,7 +780,6 @@ main(int argc, char **argv) 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, PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); + got_pathlist_free(&wanted_branches, 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 + 10d9e4239a69135e45b60ad02da2564535e8d8db --- 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, PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); free(id_str); free(id); free(refname); @@ -614,7 +610,6 @@ main(int argc, char **argv) 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, PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); + got_pathlist_free(&delete_refs, PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); 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 + 36233087a391951d9524ae108ca6272c40c8e780 --- 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, PATHLIST_FREE_NONE); return 1; } @@ -202,7 +202,7 @@ path_list_reverse_input(void) i++; } - got_pathlist_free(&paths); + got_pathlist_free(&paths, PATHLIST_FREE_NONE); return 1; } blob - d3d296f8b7c8ece84d0d00349fe8949bb2ca9eeb blob + 39f297d5af286252bd5b3fa351778e9191b7ca5f --- 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,8 @@ done: outoff++; err = add_line_metadata(lines, nlines, outoff, GOT_DIFF_LINE_NONE); done: - got_pathlist_free(&changed_paths); + got_pathlist_free(&changed_paths, + PATHLIST_FREE_PATH | PATHLIST_FREE_DATA); free(id_str); free(logmsg); free(refs_str); -- Mark Jamsek <fnc.bsdbox.org> GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68

2023-01-09 14:28 Mark Jamsek:
refactor got_pathlist_free() to optionally free path and data pointers
- 2023-01-09 14:53 Stefan Sperling:
  refactor got_pathlist_free() to optionally free path and data pointers
- - 2023-01-10 06:54 Mark Jamsek:
    refactor got_pathlist_free() to optionally free path and data pointers