Re: [RFCv2 05/16] remote.h: Change get_remote_heads return to void

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> No function uses the return value of get_remote_heads, so we don't want
> to confuse readers by it.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---

This is somewhat a sad change, as the returned value is designed to
be useful if caller wants to continue appending to the list.

Now such a caller has to tangle the list (the variable it gave the
function as the fourth argument) itself to find its tail.

Does it really "confuse" readers enough that it hurts to have a
return value?

>  connect.c | 10 ++++------
>  remote.h  |  8 ++++----
>  2 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/connect.c b/connect.c
> index 4295ba1..a2c777e 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -108,10 +108,10 @@ static void annotate_refs_with_symref_info(struct ref *ref)
>  /*
>   * Read all the refs from the other end
>   */
> -struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
> -			      struct ref **list, unsigned int flags,
> -			      struct sha1_array *extra_have,
> -			      struct sha1_array *shallow_points)
> +void get_remote_heads(int in, char *src_buf, size_t src_len,
> +		      struct ref **list, unsigned int flags,
> +		      struct sha1_array *extra_have,
> +		      struct sha1_array *shallow_points)
>  {
>  	struct ref **orig_list = list;
>  	int got_at_least_one_head = 0;
> @@ -172,8 +172,6 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
>  	}
>  
>  	annotate_refs_with_symref_info(*orig_list);
> -
> -	return list;
>  }
>  
>  static const char *parse_feature_value(struct string_list *feature_list, const char *feature, int *lenp)
> diff --git a/remote.h b/remote.h
> index 02d66ce..d5242b0 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -144,10 +144,10 @@ int check_ref_type(const struct ref *ref, int flags);
>  void free_refs(struct ref *ref);
>  
>  struct sha1_array;
> -extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
> -				     struct ref **list, unsigned int flags,
> -				     struct sha1_array *extra_have,
> -				     struct sha1_array *shallow);
> +extern void get_remote_heads(int in, char *src_buf, size_t src_len,
> +			     struct ref **list, unsigned int flags,
> +			     struct sha1_array *extra_have,
> +			     struct sha1_array *shallow);
>  
>  int resolve_remote_symref(struct ref *ref, struct ref *list);
>  int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1);