On Mon, Mar 20, 2017 at 05:33:23PM +0100, Michael Haggerty wrote:

> -/*
> - * An each_ref_entry_fn that writes the entry to a packed-refs file.
> - */
> -static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
> -{
> -     enum peel_status peel_status = peel_entry(entry, 0);
> -
> -     if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
> -             error("internal error: %s is not a valid packed reference!",
> -                   entry->name);
> -     write_packed_entry(cb_data, entry->name, entry->u.value.oid.hash,
> -                        peel_status == PEEL_PEELED ?
> -                        entry->u.value.peeled.hash : NULL);
> -     return 0;
> -}

This assertion goes away. It can't be moved into write_packed_entry()
because the peel status is only known in the caller.

But here:

> @@ -1376,8 +1362,18 @@ static int commit_packed_refs(struct files_ref_store 
> *refs)
>               die_errno("unable to fdopen packed-refs descriptor");
>  
>       fprintf_or_die(out, "%s", PACKED_REFS_HEADER);
> -     do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
> -                              write_packed_entry_fn, out);
> +
> +     iter = cache_ref_iterator_begin(packed_ref_cache->cache, NULL, 0);
> +     while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
> +             struct object_id peeled;
> +             int peel_error = ref_iterator_peel(iter, &peeled);
> +
> +             write_packed_entry(out, iter->refname, iter->oid->hash,
> +                                peel_error ? NULL : peeled.hash);
> +     }

Should we be checking that peel_error is only PEELED or NON_TAG?

-Peff

Reply via email to