Nguyễn Thái Ngọc Duy  <[email protected]> writes:

> -static void check_non_tip(void)
> +static int check_unreachable(struct object_array *src)
>  {
>...
>       /* All the non-tip ones are ancestors of what we advertised */
> -     return;
> +     return 1;
>  
>  error:
>       if (cmd.in >= 0)
>               close(cmd.in);
>       if (cmd.out >= 0)
>               close(cmd.out);
> +     return 0;
> +}

"zero is bad, one is good" is OK as a helper function that is local.
The convention needs to be documented as a comment before the
function.

Perhaps if you avoid using "check_", which does not hint the return
value, both the function and the caller would become easier to read.

How about calling it has_unreachable() and make the caller say

        if (!has_unreachable(&want_obj))
                /* all requested tips are reachable from what we advertised */
                return;

instead?

> +static void check_non_tip(void)
> +{
> +     int i;
> +
> +     /*
> +      * In the normal in-process case without
> +      * uploadpack.allowReachableSHA1InWant,
> +      * non-tip requests can never happen.
> +      */
> +     if (!stateless_rpc && !(allow_unadvertised_object_request & 
> ALLOW_REACHABLE_SHA1))
> +             goto error;
> +     if (check_unreachable(&want_obj))
> +             /* All the non-tip ones are ancestors of what we advertised */
> +             return;
> +
> +error:
>       /* Pick one of them (we know there at least is one) */
>       for (i = 0; i < want_obj.nr; i++) {
> -             o = want_obj.objects[i].item;
> +             struct object *o = want_obj.objects[i].item;
>               if (!is_our_ref(o))
>                       die("git upload-pack: not our ref %s",
>                           oid_to_hex(&o->oid));
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to