ref_resolve() leak

While I was tracking down a memory leak in ref_resolve(), I did this refactor to plug this leak in `got send`. Currently, got_send_pack() only uses got_pathlist_insert to keep a pathlist without any duplicates. The source of the leak is when a duplicate is to be inserted, got_pathlist_insert returns NULL and the caller leaks it (expecting it to be added to the list). So in order to plug this leak, we could - change the current behavior of the function (which the prototype's comment makes a point that the caller must free the memory) - pro: other function could preform the same leak (this would plug it) - con: a lot of work. - check to see if that path already exists within the pathlist before adding - pro: would prevent duplicates (plugging the leak) - con: we now are adding another linear operation - do the following Creating a new pathlist struct that is in a RB tree (rather than a TAILQ). We can use got_path_cmp() to compare each node in the tree. Since we can't have any duplicates in a RB tree, we don't need to search the tree separate from inserting into the tree. - pro: This plugs the leak. This also makes find_ref() O(log n) rather than O(n) (which may be more important once we get the bitset implementation going). - con: Slight refactor (mainly limited to send.c). Most of this code will be refactored for multi_ack support. Here is adding <sys/tree.h> diff e37a5589c16266235a9b0d3b6d7be7ec67b46390 7ebcb7fd36242f56e8e710155f09534c8da53891 commit - e37a5589c16266235a9b0d3b6d7be7ec67b46390 commit + 7ebcb7fd36242f56e8e710155f09534c8da53891 blob - 7864138db9fadd684f9cc01f99ab36941f60e2fd blob + 715d9e155d7119568bb7a61fe720c7b00036e10a --- cvg/cvg.c +++ cvg/cvg.c @@ -18,6 +18,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <sys/time.h> #include <sys/types.h> #include <sys/stat.h> blob - 8a69602e91a117f6b3b1826abfe7737979fbf616 blob + 190d9609f4e45a06227822e1dad228d0d42832a9 --- got/got.c +++ got/got.c @@ -17,6 +17,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <sys/time.h> #include <sys/types.h> #include <sys/stat.h> blob - 6678a9dd097384ee9c7b2545f4a1e9211a83054d blob + d415ed2ce1d52ef6268e933be665c9e761c8a75e --- gotadmin/gotadmin.c +++ gotadmin/gotadmin.c @@ -15,6 +15,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <sys/types.h> #include <ctype.h> blob - 31a952259533a6860e070a48fc3dfe2eed787c4f blob + cb4dac46aa51d6df7ed9d3d606dcf5100237e3d1 --- lib/commit_graph.c +++ lib/commit_graph.c @@ -17,6 +17,7 @@ #include <sys/types.h> #include <sys/stat.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/stdint.h> #include <limits.h> blob - 11a24fdcbb22ef869c3b17bf02354c6f7a580e97 blob + 5ee4248318a070e5605e27ad3ee7e408896a2ce8 --- lib/deflate.c +++ lib/deflate.c @@ -15,6 +15,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <errno.h> #include <stdio.h> blob - e43d98ae41c70e48398afaded344bc66a394b968 blob + 2c3f8c2ebcd41b76c90f5701a0b1ffc43fbdccc3 --- lib/delta.c +++ lib/delta.c @@ -15,6 +15,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <stdio.h> #include <stdlib.h> blob - 8817fd84008c7497a2a44f44f5111f20ad3414e4 blob + 22b1e2814ecb1475ebeeddd084b4e218534eae7a --- lib/dial.c +++ lib/dial.c @@ -16,6 +16,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <sys/types.h> #include <sys/socket.h> #include <sys/uio.h> blob - 80f4c2ec4a2985d8604998f23f8761ab601b2514 blob + ccd0d097b68c4dab969f27d5080ae06baa8a95f5 --- lib/diff.c +++ lib/diff.c @@ -15,6 +15,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <sys/stat.h> #include <stdio.h> blob - ca1719dd8649b956e27783774efab623fdba7ed8 blob + 8698408e92d2b39903dd77cb9a1b42118eb87457 --- lib/gitproto.c +++ lib/gitproto.c @@ -16,6 +16,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <sys/types.h> #include <ctype.h> blob - e8f06f56c03a04d123c91c39f96c6bc046cb71d9 blob + 49f05411c2efb58063883962301c09d2319d19e6 --- lib/inflate.c +++ lib/inflate.c @@ -15,6 +15,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <errno.h> #include <stdio.h> blob - 0af419be5282778934e8a5369c5793756c35ecd3 blob + 8a796d4d28606f8e51d7207a19b3cb470facba06 --- lib/lockfile.c +++ lib/lockfile.c @@ -16,6 +16,7 @@ #include <sys/stat.h> #include <sys/queue.h> +#include <sys/tree.h> #include <errno.h> #include <fcntl.h> blob - c3f64cf304de14ee32b20d2d42f888f38bec81a0 blob + f9e780471a1fdecb54382af4af19d014f1100932 --- lib/object_create.c +++ lib/object_create.c @@ -17,6 +17,7 @@ #include <sys/types.h> #include <sys/stat.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/wait.h> #include <ctype.h> blob - 8dc9adc210203ebedb74801ccf880e731d62df8b blob + ee7a9d71e39a2d65845c1ca018a7ca89d896837c --- lib/pack.c +++ lib/pack.c @@ -17,6 +17,7 @@ #include <sys/types.h> #include <sys/stat.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/uio.h> #include <sys/mman.h> #include <sys/resource.h> blob - 199fbf39d0ab1b0f251632eac6e1152e9b8d27ae blob + e59cc910d0450082aba98c17861529c605803415 --- lib/patch.c +++ lib/patch.c @@ -22,6 +22,7 @@ #include <sys/types.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/socket.h> #include <sys/stat.h> #include <sys/uio.h> blob - c91ca76066d628e1d12b7c66d86c8829e1b938be blob + edd13390e5e62ccd650fbcb1f02c92a328d766c6 --- lib/path.c +++ lib/path.c @@ -17,6 +17,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <sys/stat.h> #include <errno.h> blob - 25f3858f09dbd0bc936d97fc851892955ecf6623 blob + 5ce52b40ca0e7bb5c7565a9e9c9b02da4cc93f44 --- lib/privsep.c +++ lib/privsep.c @@ -17,6 +17,7 @@ #include <sys/types.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/uio.h> #include <sys/wait.h> blob - 3b728f8e2874054c0698accbbe24e51d69d93929 blob + 886584e46d2e919733ce2c305607c36aac37ab3e --- lib/reference.c +++ lib/reference.c @@ -16,6 +16,7 @@ #include <sys/types.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/stat.h> #include <errno.h> blob - 1855ba0c1a557ce16cfae9790d6cff44a4b71846 blob + 6ae0f96f5546d6f7eaac0b1b2d32dd6249c51d52 --- lib/worktree_open.c +++ lib/worktree_open.c @@ -16,6 +16,7 @@ #include <sys/stat.h> #include <sys/queue.h> +#include <sys/tree.h> #include <errno.h> #include <fcntl.h> blob - 920b8b12085e70e102a9c9f309d8eb555a2c9671 blob + 798d8f99cf1b256355e11bd85e7f5621ca315d39 --- libexec/got-fetch-http/got-fetch-http.c +++ libexec/got-fetch-http/got-fetch-http.c @@ -17,6 +17,7 @@ #include <sys/types.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/socket.h> #include <err.h> blob - 6ec79e85a76b5ba40ce146e249e75e24f79d7df9 blob + 8334b2d07951936cd5692acb122771a2d4e3def6 --- libexec/got-fetch-pack/got-fetch-pack.c +++ libexec/got-fetch-pack/got-fetch-pack.c @@ -16,6 +16,7 @@ #include <sys/types.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/uio.h> #include <sys/time.h> #include <sys/stat.h> blob - cc2cd7ee7882f8bf0a997b3e00e36419a15d76fc blob + b5b9532fe425af4eade0b847f25a4f4571c1281d --- libexec/got-index-pack/got-index-pack.c +++ libexec/got-index-pack/got-index-pack.c @@ -17,6 +17,7 @@ #include <sys/types.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/mman.h> #include <sys/uio.h> blob - 44ae987193ae5d6ef34fa98f645dd0ab17cb8338 blob + 41c26a054226e82d81c24bfb7d30d580dbdbff30 --- libexec/got-read-gotconfig/got-read-gotconfig.c +++ libexec/got-read-gotconfig/got-read-gotconfig.c @@ -16,6 +16,7 @@ #include <sys/types.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/uio.h> #include <sys/time.h> blob - 81b54dbe2110510c5611bd7e3319a1dabea52172 blob + 73d11a53803ce1ba875816666cb3a2c48504044e --- libexec/got-read-pack/got-read-pack.c +++ libexec/got-read-pack/got-read-pack.c @@ -17,6 +17,7 @@ #include <sys/stat.h> #include <sys/types.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/uio.h> #include <sys/time.h> #include <sys/mman.h> blob - a9cd911be347a80abbe31861aa07381dcd97ccc0 blob + c7e6d524dcf2e2e98aa97732b125828d04fb2902 --- libexec/got-read-tree/got-read-tree.c +++ libexec/got-read-tree/got-read-tree.c @@ -16,6 +16,7 @@ #include <sys/types.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/uio.h> #include <sys/time.h> blob - 2a2c22971801db07832f237f3969bc2e90a0c745 blob + dd4acd7d8447afc51816c47ab97d7a73b6376fc8 --- libexec/got-send-pack/got-send-pack.c +++ libexec/got-send-pack/got-send-pack.c @@ -17,6 +17,7 @@ #include <sys/types.h> #include <sys/queue.h> +#include <sys/tree.h> #include <sys/uio.h> #include <sys/time.h> #include <sys/stat.h> blob - 377ea9cd19e22c07f1348e7110a49defd8cb974d blob + b0b1593dc092b51883cce8fc1caf672b00d56560 --- regress/delta/delta_test.c +++ regress/delta/delta_test.c @@ -15,6 +15,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <stdio.h> #include <stdlib.h> blob - c024c69ae8a0fd18330ae5c8f6dc6b872026966e blob + e3b7eda734c43ae229647e428258d944538ab041 --- regress/fetch/fetch_test.c +++ regress/fetch/fetch_test.c @@ -15,6 +15,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <limits.h> #include <stdarg.h> blob - fad15574f54715f8f62105579fbc58c1ca5d1bf1 blob + bdf8f52499f3ff3ea50a58b637e8f5a56a31305f --- regress/path/path_test.c +++ regress/path/path_test.c @@ -15,6 +15,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <string.h> #include <stdlib.h> blob - baee88c98e9574ccb82d6365b7fe0935c8cc6a72 blob + 61793de291a3f0877e7f339d31ee856892eea43c --- tog/tog.c +++ tog/tog.c @@ -15,6 +15,7 @@ */ #include <sys/queue.h> +#include <sys/tree.h> #include <sys/stat.h> #include <sys/ioctl.h> Here is adding the new struct diff 7ebcb7fd36242f56e8e710155f09534c8da53891 33bf4ee0d21c680d725a921bdb6d67b8993b7eac commit - 7ebcb7fd36242f56e8e710155f09534c8da53891 commit + 33bf4ee0d21c680d725a921bdb6d67b8993b7eac blob - b2bcaa2b7774cab98d41dad2df3c7777d72b0d4b blob + dd20fc4db622694319973777a603d33b85c8de67 --- include/got_path.h +++ include/got_path.h @@ -147,3 +147,41 @@ const struct got_error *got_path_create_file(const cha * (Cross-mount-point moves are not yet implemented.) */ const struct got_error *got_path_move_file(const char *, const char *); + +struct got_pathtree_entry { + RB_ENTRY(got_pathtree_entry) entry; + const char *path; + size_t path_len; + void *data; +}; +int got_pathtree_cmp(const struct got_pathtree_entry * , const struct got_pathtree_entry *); + +RB_HEAD(got_pathtree, got_pathtree_entry); + + + +/* + * Insert a path into the tree of paths. + * The caller should already have initialized the tree head. This tree stores + * the pointer to the path as-is, i.e. the path is not copied internally and + * must remain available until the list is freed with got_pathlist_free(). + * If the first argument is not NULL, set it to a pointer to the newly inserted + * element, or to a NULL pointer in case the path was already on the list. + * If path to be inserted is a duplicate, then the memory of *data is freed. + */ +const struct got_error * +got_pathtree_insert(struct got_pathtree_entry **inserted, + struct got_pathtree *pathtree, const char *path, void *data); + +/* Flags passed to got_pathlist_free() to control which pointers are freed. */ +#define GOT_PATHTREE_FREE_NONE 0 /* pathlist entry only */ +#define GOT_PATHTREE_FREE_PATH (1 << 0) /* entry and path pointer */ +#define GOT_PATHTREE_FREE_DATA (1 << 1) /* entry and data pointer */ +#define GOT_PATHTREE_FREE_ALL (GOT_PATHLIST_FREE_PATH|GOT_PATHLIST_FREE_DATA) + +/* Free resources allocated for a path list. */ +void got_pathtree_free(struct got_pathtree *, int); + + + +RB_PROTOTYPE(got_pathtree, got_pathtree_entry, entry, got_pathtree_cmp); blob - 2a3413d75f8c6428f2b2737381dd72723b06f89a blob + 8fd91ca6f954b52225974d5469c085fb9eda2f4d --- lib/got_lib_privsep.h +++ lib/got_lib_privsep.h @@ -673,6 +673,7 @@ struct got_remote_repo; struct got_pack; struct got_packidx; struct got_pathlist_head; +struct got_pathtree; const struct got_error *got_send_ack(pid_t); const struct got_error *got_privsep_wait_for_child(pid_t); Finally, here is the actual refactor diff 33bf4ee0d21c680d725a921bdb6d67b8993b7eac 00932e0ce0a7f374eab1fa3ea73753d90af99626 commit - 33bf4ee0d21c680d725a921bdb6d67b8993b7eac commit + 00932e0ce0a7f374eab1fa3ea73753d90af99626 blob - 8fd91ca6f954b52225974d5469c085fb9eda2f4d blob + 3ffa8675fda6e72a906a3f706264aa01b0ccdf82 --- lib/got_lib_privsep.h +++ lib/got_lib_privsep.h @@ -715,9 +715,9 @@ const struct got_error *got_privsep_recv_fetch_progres struct got_object_id **, char **, struct got_pathlist_head *, char **, off_t *, uint8_t *, struct imsgbuf *); const struct got_error *got_privsep_send_send_req(struct imsgbuf *, int, - struct got_pathlist_head *, struct got_pathlist_head *, int); + struct got_pathtree *, struct got_pathlist_head *, int); const struct got_error *got_privsep_recv_send_remote_refs( - struct got_pathlist_head *, struct imsgbuf *); + struct got_pathtree *, struct imsgbuf *); const struct got_error *got_privsep_send_packfd(struct imsgbuf *, int); const struct got_error *got_privsep_recv_send_progress(int *, off_t *, int *, char **, char **, struct imsgbuf *); blob - edd13390e5e62ccd650fbcb1f02c92a328d766c6 blob + b1c77c02b38c238932bf9e487508769ffb63de01 --- lib/path.c +++ lib/path.c @@ -38,6 +38,7 @@ #define MIN(_a,_b) ((_a) < (_b) ? (_a) : (_b)) #endif + int got_path_is_absolute(const char *path) { @@ -214,7 +215,58 @@ got_path_cmp(const char *path1, const char *path2, siz return (unsigned char)path1[i] < (unsigned char)path2[i] ? -1 : 1; } +int +got_pathtree_cmp(const struct got_pathtree_entry *p1, const struct got_pathtree_entry *p2) +{ + return got_path_cmp(p1->path, p2->path, p1->path_len, p2->path_len); +} + const struct got_error * +got_pathtree_insert(struct got_pathtree_entry **inserted, + struct got_pathtree *pathlist, const char *path, void *data) +{ + struct got_pathtree_entry *new; + size_t path_len = strlen(path); + + if (inserted) + *inserted = NULL; + + new = malloc(sizeof(*new)); + if (new == NULL) + return got_error_from_errno("malloc"); + new->path = path; + new->path_len = path_len; + new->data = data; + if(RB_INSERT(got_pathtree, pathlist, new)){ + free(data); + free(new); + return NULL; + } + if (inserted) + *inserted = new; + return NULL; +} + +void +got_pathtree_free(struct got_pathtree *tree, int freemask) +{ + struct got_pathtree_entry *te, *temp; + RB_FOREACH_SAFE(te, got_pathtree, tree, temp) { + if (freemask & GOT_PATHLIST_FREE_PATH) { + free((char *)te->path); + te->path = NULL; + } + if (freemask & GOT_PATHLIST_FREE_DATA) { + free(te->data); + te->data = NULL; + } + RB_REMOVE(got_pathtree, tree, te); + free(te); + } +} + + +const struct got_error * got_pathlist_insert(struct got_pathlist_entry **inserted, struct got_pathlist_head *pathlist, const char *path, void *data) { @@ -291,6 +343,7 @@ got_pathlist_free(struct got_pathlist_head *pathlist, } } + static const struct got_error * make_parent_dirs(const char *abspath) { @@ -566,3 +619,5 @@ got_path_move_file(const char *oldpath, const char *ne return NULL; } + +RB_GENERATE(got_pathtree, got_pathtree_entry, entry, got_pathtree_cmp); blob - 5ce52b40ca0e7bb5c7565a9e9c9b02da4cc93f44 blob + ac0e9184a9045ed70bf88a6cc4e2dc3e47c8fe05 --- lib/privsep.c +++ lib/privsep.c @@ -885,19 +885,20 @@ send_send_ref(const char *name, size_t name_len, struc const struct got_error * got_privsep_send_send_req(struct imsgbuf *ibuf, int fd, - struct got_pathlist_head *have_refs, + struct got_pathtree *have_refs, struct got_pathlist_head *delete_refs, int verbosity) { const struct got_error *err = NULL; struct got_pathlist_entry *pe; + struct got_pathtree_entry *temp_pe; struct got_imsg_send_request sendreq; struct got_object_id zero_id; memset(&zero_id, 0, sizeof(zero_id)); memset(&sendreq, 0, sizeof(sendreq)); sendreq.verbosity = verbosity; - TAILQ_FOREACH(pe, have_refs, entry) + RB_FOREACH(temp_pe, got_pathtree ,have_refs) sendreq.nrefs++; TAILQ_FOREACH(pe, delete_refs, entry) sendreq.nrefs++; @@ -913,13 +914,13 @@ got_privsep_send_send_req(struct imsgbuf *ibuf, int fd if (err) goto done; - TAILQ_FOREACH(pe, have_refs, entry) { - const char *name = pe->path; - size_t name_len = pe->path_len; - struct got_object_id *id = pe->data; + RB_FOREACH(temp_pe, got_pathtree, have_refs) { + const char *name = temp_pe->path; + size_t name_len = temp_pe->path_len; + struct got_object_id *id = temp_pe->data; err = send_send_ref(name, name_len, id, 0, ibuf); if (err) - goto done; + goto done; } TAILQ_FOREACH(pe, delete_refs, entry) { @@ -936,7 +937,7 @@ done: } const struct got_error * -got_privsep_recv_send_remote_refs(struct got_pathlist_head *remote_refs, +got_privsep_recv_send_remote_refs(struct got_pathtree *remote_refs, struct imsgbuf *ibuf) { const struct got_error *err = NULL; @@ -945,7 +946,7 @@ got_privsep_recv_send_remote_refs(struct got_pathlist_ struct got_imsg_send_remote_ref iremote_ref; struct got_object_id *id = NULL; char *refname = NULL; - struct got_pathlist_entry *new; + struct got_pathtree_entry *new; while (1) { err = got_privsep_recv_imsg(&imsg, ibuf, 0); @@ -976,7 +977,7 @@ got_privsep_recv_send_remote_refs(struct got_pathlist_ err = got_error_from_errno("strndup"); goto done; } - err = got_pathlist_insert(&new, remote_refs, + err = got_pathtree_insert(&new, remote_refs, refname, id); if (err) goto done; blob - 1976fa9212d7b93d58eab0f3844f52e5f8d48f06 blob + fafedac2b346b4f2a79b9207dc0ac32cd8ef69da --- lib/send.c +++ lib/send.c @@ -166,7 +166,7 @@ pack_progress(void *arg, int ncolored, int nfound, int } static const struct got_error * -insert_sendable_ref(struct got_pathlist_head *refs, const char *refname, +insert_sendable_ref(struct got_pathtree *refs, const char *refname, const char *target_refname, struct got_repository *repo) { const struct got_error *err; @@ -200,7 +200,7 @@ insert_sendable_ref(struct got_pathlist_head *refs, co goto done; } - err = got_pathlist_insert(NULL, refs, target_refname, id); + err = got_pathtree_insert(NULL, refs, target_refname, id); done: if (ref) got_ref_close(ref); @@ -258,19 +258,14 @@ realloc_ids(struct got_object_id ***ids, size_t *nallo return NULL; } -static struct got_pathlist_entry * -find_ref(struct got_pathlist_head *refs, const char *refname) +static struct got_pathtree_entry * +find_ref(struct got_pathtree *refs, const char *refname) { - struct got_pathlist_entry *pe; - - TAILQ_FOREACH(pe, refs, entry) { - if (got_path_cmp(pe->path, refname, strlen(pe->path), - strlen(refname)) == 0) { - return pe; - } - } - - return NULL; + struct got_pathtree_entry find; + find.path = refname; + find.path_len = strlen(refname); + return RB_FIND(got_pathtree, refs, &find); + } static const struct got_error * @@ -290,7 +285,7 @@ get_remote_refname(char **remote_refname, const char * } static const struct got_error * -update_remote_ref(struct got_pathlist_entry *my_ref, const char *remote_name, +update_remote_ref(struct got_pathtree_entry *my_ref, const char *remote_name, struct got_repository *repo) { const struct got_error *err, *unlock_err; @@ -346,8 +341,9 @@ got_send_pack(const char *remote_name, struct got_path const struct got_error *err; struct imsgbuf sendibuf; pid_t sendpid = -1; - struct got_pathlist_head have_refs; - struct got_pathlist_head their_refs; + struct got_pathtree their_refs; + struct got_pathtree have_refs; + struct got_pathtree_entry *te = NULL; struct got_pathlist_entry *pe; struct got_object_id **our_ids = NULL; struct got_object_id **their_ids = NULL; @@ -361,8 +357,8 @@ got_send_pack(const char *remote_name, struct got_path FILE *delta_cache = NULL; char *s = NULL; - TAILQ_INIT(&have_refs); - TAILQ_INIT(&their_refs); + RB_INIT(&have_refs); + RB_INIT(&their_refs); if (got_repo_get_object_format(repo) != GOT_HASH_SHA1) return got_error_fmt(GOT_ERR_NOT_IMPL, @@ -394,19 +390,20 @@ got_send_pack(const char *remote_name, struct got_path TAILQ_FOREACH(pe, delete_branches, entry) { const char *branchname = pe->path; - struct got_pathlist_entry *ref; + struct got_pathtree_entry *temp_ref; if (strncmp(branchname, "refs/heads/", 11) != 0) { err = got_error_fmt(GOT_ERR_SEND_DELETE_REF, "%s", branchname); goto done; } - ref = find_ref(&have_refs, branchname); - if (ref) { + temp_ref = find_ref(&have_refs, branchname); + if (temp_ref) { err = got_error_fmt(GOT_ERR_SEND_DELETE_REF, - "changes on %s will be sent to server", - branchname); + "changes on %s will be sent to server", + branchname); goto done; - } + } + } TAILQ_FOREACH(pe, tag_names, entry) { @@ -425,10 +422,11 @@ got_send_pack(const char *remote_name, struct got_path err = insert_sendable_ref(&have_refs, s, s, repo); if (err) goto done; + s = NULL; } - if (TAILQ_EMPTY(&have_refs) && TAILQ_EMPTY(delete_branches)) { + if (RB_EMPTY(&have_refs) && TAILQ_EMPTY(delete_branches)) { err = got_error(GOT_ERR_SEND_EMPTY); goto done; } @@ -474,9 +472,8 @@ got_send_pack(const char *remote_name, struct got_path * Prepare the array of our object IDs which * will be needed for generating a pack file. */ - TAILQ_FOREACH(pe, &have_refs, entry) { - struct got_object_id *id = pe->data; - + RB_FOREACH(te, got_pathtree, &have_refs){ + struct got_object_id *id = te->data; err = realloc_ids(&our_ids, &nalloc_ours, nours + 1); if (err) goto done; @@ -499,12 +496,12 @@ got_send_pack(const char *remote_name, struct got_path * This array will be used to exclude objects which already * exist on the server from our pack file. */ - TAILQ_FOREACH(pe, &their_refs, entry) { - const char *refname = pe->path; - struct got_object_id *their_id = pe->data; + RB_FOREACH(te, got_pathtree, &their_refs) { + const char *refname = te->path; + struct got_object_id *their_id = te->data; int have_their_id; struct got_object *obj; - struct got_pathlist_entry *my_ref = NULL; + struct got_pathtree_entry *my_ref = NULL; int is_tag = 0; /* Don't blindly trust the server to send us valid names. */ @@ -592,8 +589,8 @@ got_send_pack(const char *remote_name, struct got_path } /* Account for any new references we are going to upload. */ - TAILQ_FOREACH(pe, &have_refs, entry) { - const char *refname = pe->path; + RB_FOREACH(te, got_pathtree, &have_refs){ + const char *refname = te->path; if (find_ref(&their_refs, refname) == NULL) refs_to_send++; } @@ -650,15 +647,17 @@ got_send_pack(const char *remote_name, struct got_path } err = got_privsep_recv_send_progress(&done, &bytes_sent, &success, &refname, &errmsg, &sendibuf); - if (err) + if (err) goto done; if (refname && got_ref_name_is_valid(refname) && success && strncmp(refname, "refs/tags/", 10) != 0) { - struct got_pathlist_entry *my_ref; + struct got_pathtree_entry *my_ref; + /* * The server has accepted our changes. * Update our reference in refs/remotes/ accordingly. */ + my_ref = find_ref(&have_refs, refname); if (my_ref) { err = update_remote_ref(my_ref, remote_name, @@ -700,8 +699,9 @@ done: if (npackfd != -1 && close(npackfd) == -1 && err == NULL) err = got_error_from_errno("close"); - got_pathlist_free(&have_refs, GOT_PATHLIST_FREE_ALL); - got_pathlist_free(&their_refs, GOT_PATHLIST_FREE_ALL); + got_pathtree_free(&have_refs, GOT_PATHTREE_FREE_ALL); + got_pathtree_free(&their_refs, GOT_PATHTREE_FREE_ALL); + /* * Object ids are owned by have_refs/their_refs and are already freed; * Only the arrays must be freed. If this is something wanted, I can look for other spots to use this new struct. All regressions have passed on my end. Thoughts/comments/sugguestions?

2024-11-10 17:01 Kyle Ackerman:
ref_resolve() leak
- 2024-11-11 23:13 Stefan Sperling:
  ref_resolve() leak
- - 2024-11-12 06:43 Kyle Ackerman:
    ref_resolve() leak