Re: [PATCH v2 25/25] refs: document the lifetime of the args passed to each_ref_fn

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

Michael Haggerty <mhagger@alum.mit.edu> writes:

> The commits leading up to this have (hopefully) fixed all of the
> callers of the for_each_ref()-like functions.  This commit does the
> last step: documents what each_ref_fn callbacks can assume about
> object lifetimes.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>

I looked at all the patches in the series and they all looked
sensible.

I have a few comments (sent as individual review) but all of the
suggestions in them are "by the way I noticed this issue that is not
new to this series while I was reading the code, and we may want to
fix it elsewhere", not "this is broken in the patch, we need to fix
it before queuing".

Thanks for a pleasant read.

> ---
>  refs.h | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/refs.h b/refs.h
> index a35eafc..122ec03 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -15,13 +15,23 @@ struct ref_lock {
>  #define REF_ISBROKEN 0x04
>  
>  /*
> - * Calls the specified function for each ref file until it returns
> - * nonzero, and returns the value.  Please note that it is not safe to
> - * modify references while an iteration is in progress, unless the
> - * same callback function invocation that modifies the reference also
> - * returns a nonzero value to immediately stop the iteration.
> + * The signature for the callback function for the for_each_*()
> + * functions below.  The memory pointed to by the refname and sha1
> + * arguments is only guaranteed to be valid for the duration of a
> + * single callback invocation.
> + */
> +typedef int each_ref_fn(const char *refname,
> +			const unsigned char *sha1, int flags, void *cb_data);
> +
> +/*
> + * The following functions invoke the specified callback function for
> + * each reference indicated.  If the function ever returns a nonzero
> + * value, stop the iteration and return that value.  Please note that
> + * it is not safe to modify references while an iteration is in
> + * progress, unless the same callback function invocation that
> + * modifies the reference also returns a nonzero value to immediately
> + * stop the iteration.
>   */
> -typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int flags, void *cb_data);
>  extern int head_ref(each_ref_fn, void *);
>  extern int for_each_ref(each_ref_fn, void *);
>  extern int for_each_ref_in(const char *, each_ref_fn, void *);