Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
> It is a nice unit of work and soon will be needed from multiple
> locations.
>
> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
> ---
> refs.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index c523978..dfc8600 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -529,6 +529,22 @@ static void sort_ref_dir(struct ref_dir *dir)
> #define DO_FOR_EACH_INCLUDE_BROKEN 0x01
>
> /*
> + * Return true iff the reference described by entry can be resolved to
> + * an object in the database. Emit a warning if the referred-to
> + * object does not exist.
> + */
> +static int ref_resolves_to_object(struct ref_entry *entry)
> +{
> + if (entry->flag & REF_ISBROKEN)
> + return 0;
> + if (!has_sha1_file(entry->u.value.sha1)) {
> + error("%s does not point to a valid object!", entry->name);
> + return 0;
> + }
> + return 1;
> +}
> +
> +/*
> * current_ref is a performance hack: when iterating over references
> * using the for_each_ref*() functions, current_ref is set to the
> * current reference's entry before calling the callback function. If
> @@ -549,14 +565,10 @@ static int do_one_ref(const char *base, each_ref_fn fn, int trim,
> if (prefixcmp(entry->name, base))
> return 0;
>
> - if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) {
> - if (entry->flag & REF_ISBROKEN)
> - return 0; /* ignore broken refs e.g. dangling symref */
> - if (!has_sha1_file(entry->u.value.sha1)) {
> - error("%s does not point to a valid object!", entry->name);
> - return 0;
> - }
> - }
> + if (!((flags & DO_FOR_EACH_INCLUDE_BROKEN) ||
> + ref_resolves_to_object(entry)))
> + return 0;
> +
The original says "Unless flags tell us to include broken ones,
return 0 for the broken ones and the ones that point at invalid
objects".
The updated says "Unless flags tell us to include broken ones or the
ref is a good one, return 0".
Are they the same? If we have a good one, and if we are not told to
include broken one, the original just passes the control down to the
remainder of the function. The updated one will return 0.
Oh, wait, that is not the case. The OR is inside !( A || B ), so
what it really wants to say is:
if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN) &&
!ref_resolves_to_object(entry))
return 0;
that is, "When we are told to exclude broken ones and the one we are
looking at is broken, return 0".
Am I the only one who was confused with this, or was the way the
patch wrote this logic unnecessarily convoluted?
> current_ref = entry;
> retval = fn(entry->name + trim, entry->u.value.sha1, entry->flag, cb_data);
> current_ref = NULL;
--
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