On Thu, Feb 4, 2016 at 4:04 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote: > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> > --- > diff --git a/upload-pack.c b/upload-pack.c > @@ -451,8 +451,16 @@ static int is_our_ref(struct object *o) > -static int check_unreachable(struct object_array *src) > +/* > + * If reachable is NULL, return 1 if there is no unreachable object, > + * zero otherwise. > + * > + * If reachable is not NULL, it's filled with reachable objects. > + * Return value is irrelevant. The caller has to compare reachable and > + * src to find out if there's any unreachable object. > + */ This dual-purpose function serving up two entirely orthogonal modes feel strange. Would it make sense to split this into two functions, one for each paragraph in the above documentation? (Of course, they could share common implementation behind-the-scenes as needed.) More below... > +static int check_unreachable(struct object_array *reachable, > + struct object_array *src) > { > static const char *argv[] = { > "rev-list", "--stdin", NULL, > @@ -507,9 +522,31 @@ static int check_unreachable(struct object_array *src) > * The commits out of the rev-list are not ancestors of > * our ref. > */ > - i = read_in_full(cmd.out, namebuf, 1); > - if (i) > - return 0; > + if (!reachable) { > + i = read_in_full(cmd.out, namebuf, 1); > + if (i) > + return 0; > + } else { > + while ((i = read_in_full(cmd.out, namebuf, 41)) == 41) { > + struct object_id sha1; > + > + if (namebuf[40] != '\n' || get_oid_hex(namebuf, &sha1)) > + break; > + > + o = lookup_object(sha1.hash); > + if (o && o->type == OBJ_COMMIT) { > + o->flags &= ~TMP_MARK; > + } > + } > + for (i = get_max_object_index(); 0 < i; ) { > + o = get_indexed_object(--i); Perhaps code this as: for (i = get_max_object_index(); 0 < i; i--) { o = get_indexed_object(i - 1); in order to keep the loop control within the 'for' itself? > + if (o && o->type == OBJ_COMMIT && > + (o->flags & TMP_MARK)) { > + add_object_array(o, NULL, reachable); > + o->flags &= ~TMP_MARK; > + } > + } > + } -- 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