On Fri, Jun 3, 2016 at 5:03 PM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> We want ref_stores to be polymorphic, so invent a base class of which
> files_ref_store is a derived class. For now there is a one-to-one
> relationship between ref_stores and submodules.
>
> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
> ---
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> @@ -973,53 +967,54 @@ static void clear_loose_ref_cache(struct files_ref_store *refs)
> +/*
> + * Downcast ref_store to files_ref_store. Die if ref_store is not a
> + * files_ref_store. If submodule_allowed is not true, then also die if
> + * files_ref_store is for a submodule (i.e., not for the main
> + * repository). caller is used in any necessary error messages.
> + */
> +static struct files_ref_store *files_downcast(
> + struct ref_store *ref_store, int submodule_allowed,
> + const char *caller)
> {
> struct files_ref_store *refs;
>
> + if (ref_store->be != &refs_be_files)
> + die("BUG: ref_store is type \"%s\" not \"files\" in %s",
> + ref_store->be->name, caller);
>
> + refs = (struct files_ref_store *)ref_store;
> +
> + if (!submodule_allowed)
> + assert_main_repository(ref_store, caller);
> +
> + return refs;
> }
Aside from returning the downcasted value, 'refs' doesn't seem to be
used for anything, thus could be dropped and the downcasted value
returned directly:
return (struct files_ref_store *)ref_store;
Not worth a re-roll.
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> @@ -521,11 +521,89 @@ int read_raw_ref(const char *refname, unsigned char *sha1,
> +/*
> + * A representation of the reference store for the main repository or
> + * a submodule. The ref_store instances for submodules are kept in a
> + * linked list.
> + */
> +struct ref_store {
> + /* The backend describing this ref_store's storage scheme: */
> + const struct ref_storage_be *be;
> +
> + /*
> + * The name of the submodule represented by this object, or
> + * the empty string if it represents the main repository's
> + * reference store:
> + */
> + const char *submodule;
Tangent: Apart from backward compatibility due to all the existing
code which tests *submodule to distinguish between the main repository
and a submodule, is there a technical reason that this ought to store
an empty string rather than (the more idiomatic) NULL to signify the
main repository?
--
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