Subject: Re: [PATCH 01/16] foreach_alt_odb: propagate return value from callback

From: René Scharfe <[email-masked]>
Date: Sat, 04 Oct 2014 00:55:13 +0200

Am 03.10.2014 um 22:21 schrieb Jeff King:
> We check the return value of the callback and stop iterating
> if it is non-zero. However, we do not make the non-zero
> return value available to the caller, so they have no way of
> knowing whether the operation succeeded or not (technically
> they can keep their own error flag in the callback data, but
> that is unlike our other for_each functions).
>
> Signed-off-by: Jeff King <[email-masked]>
> ---
>   cache.h     |  2 +-
>   sha1_file.c | 12 ++++++++----
>   2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 8206039..cd16e25 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1143,7 +1143,7 @@ extern void prepare_alt_odb(void);
>   extern void read_info_alternates(const char * relative_base, int depth);
>   extern void add_to_alternates_file(const char *reference);
>   typedef int alt_odb_fn(struct alternate_object_database *, void *);
> -extern void foreach_alt_odb(alt_odb_fn, void*);
> +extern int foreach_alt_odb(alt_odb_fn, void*);
>
>   struct pack_window {
>   	struct pack_window *next;
> diff --git a/sha1_file.c b/sha1_file.c
> index c08c0cb..bae1c15 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -412,14 +412,18 @@ void add_to_alternates_file(const char *reference)
>   		link_alt_odb_entries(alt, strlen(alt), '\n', NULL, 0);
>   }
>
> -void foreach_alt_odb(alt_odb_fn fn, void *cb)
> +int foreach_alt_odb(alt_odb_fn fn, void *cb)
>   {
>   	struct alternate_object_database *ent;
> +	int r = 0;
>
>   	prepare_alt_odb();
> -	for (ent = alt_odb_list; ent; ent = ent->next)
> -		if (fn(ent, cb))
> -			return;
> +	for (ent = alt_odb_list; ent; ent = ent->next) {
> +		int r = fn(ent, cb);
> +		if (r)
> +			break;
> +	}
> +	return r;

This will always return zero.  You probably shadowed r unintentionally 
inside the loop, right?

>   }
>
>   void prepare_alt_odb(void)
>