Re: [PATCH 4/6] repack_without_refs(): make the refnames argument a string_list
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Nov 22, 2014, 21:17 UTC
- Message-ID
- <20141122211722.GD15320@google.com>
- In-Reply-To
- <1416578950-23210-5-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty wrote:
> All of the callers have string_lists available already
Technically ref_transaction_commit doesn't, but that doesn't matter.
Show 7 quoted lines
> Suggested-by: Ronnie Sahlberg <sahlberg@google.com> > Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> > --- > builtin/remote.c | 14 ++------------ > refs.c | 38 ++++++++++++++++++++------------------ > refs.h | 11 ++++++++++- > 3 files changed, 32 insertions(+), 31 deletions(-)
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
One (optional) nit at the bottom of this message.
[...]
Show 30 quoted lines
> +++ b/refs.c
> @@ -2639,22 +2639,25 @@ 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 *refnames, 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;
> + struct string_list_item *refname, *ref_to_delete;
> + int ret, needs_repacking = 0, removed = 0;
>
> assert(err);
>
> /* Look for a packed ref */
> - for (i = 0; i < n; i++)
> - if (get_packed_ref(refnames[i]))
> + for_each_string_list_item(refname, refnames) {
> + if (get_packed_ref(refname->string)) {
> + needs_repacking = 1;
> break;
> + }
> + }
>
> /* Avoid locking if we have nothing to do */
> - if (i == n)
> + if (!needs_repacking)This makes me wish C supported something like Python's for/else construct. Oh well. :)
[...]
Show 14 quoted lines
> +++ b/refs.h > @@ -163,7 +163,16 @@ extern void rollback_packed_refs(void); > */ > int pack_refs(unsigned int flags); > > -extern int repack_without_refs(const char **refnames, int n, > +/* > + * Remove the refs listed in 'refnames' from the packed-refs file. > + * On error, packed-refs will be unchanged, the return value is > + * nonzero, and a message about the error is written to the 'err' > + * strbuf. > + * > + * The refs in 'refnames' needn't be sorted. The err buffer must not be > + * omitted.
(nit)
s/buffer/strbuf/, or s/The err buffer/'err'/ s/omitted/NULL/
Thanks, Jonathan