Stefan Beller <sbeller@xxxxxxxxxx> writes: > This patch was heavily inspired by a part of the ref-transactions-rename > series[1], but people tend to dislike large series and this part is > relatively easy to take out and unrelated, so I'll send it as a single > patch. > > This patch doesn't intend any functional changes. It is just a refactoring, > which replaces a char** array by a stringlist in the function > repack_without_refs. > > [1] https://www.mail-archive.com/git@xxxxxxxxxxxxxxx/msg60604.html > > Idea-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx> > Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx> > --- > builtin/remote.c | 22 +++++++--------------- > refs.c | 41 ++++++++++++++++++++--------------------- > refs.h | 3 +-- > 3 files changed, 28 insertions(+), 38 deletions(-) In one codepath we were already using a string_list delete_refs_list anyway, so it makes sense to reuse that by movingan existing call to string_list_insert() a bit higher, instead of maintaining another array of pointers delete_refs[] to strings. OK, it simplifies the code by reducing the line count, which is a plus ;-) Sounds good. > > diff --git a/builtin/remote.c b/builtin/remote.c > index 7f28f92..dca4ebf 100644 > --- a/builtin/remote.c > +++ b/builtin/remote.c > @@ -750,16 +750,11 @@ static int mv(int argc, const char **argv) > static int remove_branches(struct string_list *branches) > { > struct strbuf err = STRBUF_INIT; > - const char **branch_names; > int i, result = 0; > > - branch_names = xmalloc(branches->nr * sizeof(*branch_names)); > - for (i = 0; i < branches->nr; i++) > - branch_names[i] = branches->items[i].string; > - if (repack_without_refs(branch_names, branches->nr, &err)) > + if (repack_without_refs(branches, &err)) > result |= error("%s", err.buf); > strbuf_release(&err); > - free(branch_names); > > for (i = 0; i < branches->nr; i++) { > struct string_list_item *item = branches->items + i; > @@ -1317,7 +1312,6 @@ static int prune_remote(const char *remote, int dry_run) > int result = 0, i; > struct ref_states states; > struct string_list delete_refs_list = STRING_LIST_INIT_NODUP; > - const char **delete_refs; > const char *dangling_msg = dry_run > ? _(" %s will become dangling!") > : _(" %s has become dangling!"); > @@ -1325,6 +1319,11 @@ static int prune_remote(const char *remote, int dry_run) > memset(&states, 0, sizeof(states)); > get_remote_ref_states(remote, &states, GET_REF_STATES); > > + for (i = 0; i < states.stale.nr; i++) > + string_list_insert(&delete_refs_list, > + states.stale.items[i].util); > + > + > if (states.stale.nr) { > printf_ln(_("Pruning %s"), remote); > printf_ln(_("URL: %s"), > @@ -1332,24 +1331,17 @@ static int prune_remote(const char *remote, int dry_run) > ? states.remote->url[0] > : _("(no URL)")); > > - delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs)); > - for (i = 0; i < states.stale.nr; i++) > - delete_refs[i] = states.stale.items[i].util; > if (!dry_run) { > struct strbuf err = STRBUF_INIT; > - if (repack_without_refs(delete_refs, states.stale.nr, > - &err)) > + if (repack_without_refs(&delete_refs_list, &err)) > result |= error("%s", err.buf); > strbuf_release(&err); > } > - free(delete_refs); > } > > for (i = 0; i < states.stale.nr; i++) { > const char *refname = states.stale.items[i].util; > > - string_list_insert(&delete_refs_list, refname); > - > if (!dry_run) > result |= delete_ref(refname, NULL, 0); > > diff --git a/refs.c b/refs.c > index 5ff457e..2333a9b 100644 > --- a/refs.c > +++ b/refs.c > @@ -2639,23 +2639,23 @@ static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data) > return 0; > } > > -int repack_without_refs(const char **refnames, int n, struct strbuf *err) > +int repack_without_refs(struct string_list *without, struct strbuf *err) > { > struct ref_dir *packed; > struct string_list refs_to_delete = STRING_LIST_INIT_DUP; > struct string_list_item *ref_to_delete; > - int i, ret, removed = 0; > + int count, ret, removed = 0; > > assert(err); > > - /* Look for a packed ref */ > - for (i = 0; i < n; i++) > - if (get_packed_ref(refnames[i])) > - break; > + count = 0; > + for_each_string_list_item(ref_to_delete, without) > + if (get_packed_ref(ref_to_delete->string)) > + count++; > > - /* Avoid locking if we have nothing to do */ > - if (i == n) > - return 0; /* no refname exists in packed refs */ > + /* No refname exists in packed refs */ > + if (!count) > + return 0; > > if (lock_packed_refs(0)) { > unable_to_lock_message(git_path("packed-refs"), errno, err); > @@ -2664,8 +2664,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err) > packed = get_packed_refs(&ref_cache); > > /* Remove refnames from the cache */ > - for (i = 0; i < n; i++) > - if (remove_entry(packed, refnames[i]) != -1) > + for_each_string_list_item(ref_to_delete, without) > + if (remove_entry(packed, ref_to_delete->string) != -1) > removed = 1; > if (!removed) { > /* > @@ -3738,10 +3738,11 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n, > int ref_transaction_commit(struct ref_transaction *transaction, > struct strbuf *err) > { > - int ret = 0, delnum = 0, i; > - const char **delnames; > + int ret = 0, i; > int n = transaction->nr; > struct ref_update **updates = transaction->updates; > + struct string_list refs_to_delete = STRING_LIST_INIT_DUP; > + struct string_list_item *ref_to_delete; > > assert(err); > > @@ -3753,9 +3754,6 @@ int ref_transaction_commit(struct ref_transaction *transaction, > return 0; > } > > - /* Allocate work space */ > - delnames = xmalloc(sizeof(*delnames) * n); > - > /* Copy, sort, and reject duplicate refs */ > qsort(updates, n, sizeof(*updates), ref_update_compare); > if (ref_update_reject_duplicates(updates, n, err)) { > @@ -3815,16 +3813,17 @@ int ref_transaction_commit(struct ref_transaction *transaction, > } > > if (!(update->flags & REF_ISPRUNING)) > - delnames[delnum++] = update->lock->ref_name; > + string_list_insert(&refs_to_delete, > + update->lock->ref_name); > } > } > > - if (repack_without_refs(delnames, delnum, err)) { > + if (repack_without_refs(&refs_to_delete, err)) { > ret = TRANSACTION_GENERIC_ERROR; > goto cleanup; > } > - for (i = 0; i < delnum; i++) > - unlink_or_warn(git_path("logs/%s", delnames[i])); > + for_each_string_list_item(ref_to_delete, &refs_to_delete) > + unlink_or_warn(git_path("logs/%s", ref_to_delete->string)); > clear_loose_ref_cache(&ref_cache); > > cleanup: > @@ -3833,7 +3832,7 @@ cleanup: > for (i = 0; i < n; i++) > if (updates[i]->lock) > unlock_ref(updates[i]->lock); > - free(delnames); > + string_list_clear(&refs_to_delete, 0); > return ret; > } > > diff --git a/refs.h b/refs.h > index 2bc3556..0416e5f 100644 > --- a/refs.h > +++ b/refs.h > @@ -163,8 +163,7 @@ extern void rollback_packed_refs(void); > */ > int pack_refs(unsigned int flags); > > -extern int repack_without_refs(const char **refnames, int n, > - struct strbuf *err); > +extern int repack_without_refs(struct string_list *without, struct strbuf *err); > > extern int ref_exists(const char *); -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html