Re: [PATCH 12/16] sha1_file: add for_each iterators for loose and packed objects

["René Scharfe" <l.s.r@web.de>]

Am 03.10.2014 um 22:32 schrieb Jeff King:
> We typically iterate over the reachable objects in a
> repository by starting at the tips and walking the graph.
> There's no easy way to iterate over all of the objects,
> including unreachable ones. Let's provide a way of doing so.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>   cache.h     | 11 +++++++++++
>   sha1_file.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 73 insertions(+)
>
> diff --git a/cache.h b/cache.h
> index 7abe7f6..3826b4b 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1270,6 +1270,17 @@ int for_each_loose_file_in_objdir(const char *path,
>   				  each_loose_subdir_fn subdir_cb,
>   				  void *data);
>
> +/*
> + * Iterate over loose and packed objects in both the local
> + * repository and any alternates repositories.
> + */
> +typedef int each_packed_object_fn(const unsigned char *sha1,
> +				  struct packed_git *pack,
> +				  uint32_t pos,
> +				  void *data);
> +extern int for_each_loose_object(each_loose_object_fn, void *);
> +extern int for_each_packed_object(each_packed_object_fn, void *);
> +
>   struct object_info {
>   	/* Request */
>   	enum object_type *typep;
> diff --git a/sha1_file.c b/sha1_file.c
> index 9fdad47..d017289 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -3313,3 +3313,65 @@ int for_each_loose_file_in_objdir(const char *path,
>   	strbuf_release(&buf);
>   	return r;
>   }
> +
> +struct loose_alt_odb_data {
> +	each_loose_object_fn *cb;
> +	void *data;
> +};
> +
> +static int loose_from_alt_odb(struct alternate_object_database *alt,
> +			      void *vdata)
> +{
> +	struct loose_alt_odb_data *data = vdata;
> +	return for_each_loose_file_in_objdir(alt->base,
> +					     data->cb, NULL, NULL,
> +					     data->data);
> +}
> +
> +int for_each_loose_object(each_loose_object_fn cb, void *data)
> +{
> +	struct loose_alt_odb_data alt;
> +	int r;
> +
> +	r = for_each_loose_file_in_objdir(get_object_directory(),
> +					  cb, NULL, NULL, data);
> +	if (r)
> +		return r;
> +
> +	alt.cb = cb;
> +	alt.data = data;
> +	return foreach_alt_odb(loose_from_alt_odb, &alt);
> +}
> +
> +int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn cb, void *data)

Should this one be declared static?  It seems to be used only in 
sha1_file.c.

> +{
> +	uint32_t i;
> +	int r = 0;
> +
> +	for (i = 0; i < p->num_objects; i++) {
> +		const unsigned char *sha1 = nth_packed_object_sha1(p, i);
> +
> +		if (!sha1)
> +			return error("unable to get sha1 of object %u in %s",
> +				     i, p->pack_name);
> +
> +		r = cb(sha1, p, i, data);
> +		if (r)
> +			break;
> +	}
> +	return r;
> +}
> +
> +int for_each_packed_object(each_packed_object_fn cb, void *data)
> +{
> +	struct packed_git *p;
> +	int r = 0;
> +
> +	prepare_packed_git();
> +	for (p = packed_git; p; p = p->next) {
> +		r = for_each_object_in_pack(p, cb, data);
> +		if (r)
> +			break;
> +	}
> +	return 0;
> +}

Perhaps return r instead here?

René