On Wed, Apr 13, 2016 at 8:55 AM, Nguyễn Thái Ngọc Duy <[email protected]> wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy <[email protected]>
> ---
> diff --git a/upload-pack.c b/upload-pack.c
> @@ -452,24 +452,24 @@ static int is_our_ref(struct object *o)
> -static int check_unreachable(struct object_array *src)
> +static int do_reachable_revlist(struct child_process *cmd,
> + struct object_array *src)
> @@ -487,8 +487,8 @@ static int check_unreachable(struct object_array *src)
> if (!is_our_ref(o))
> continue;
> memcpy(namebuf + 1, oid_to_hex(&o->oid), GIT_SHA1_HEXSZ);
> - if (write_in_full(cmd.in, namebuf, 42) < 0)
> - return 0;
> + if (write_in_full(cmd->in, namebuf, 42) < 0)
> + return -1;
> }
> namebuf[40] = '\n';
> for (i = 0; i < src->nr; i++) {
> @@ -496,18 +496,29 @@ static int check_unreachable(struct object_array *src)
> if (is_our_ref(o))
> continue;
> memcpy(namebuf, oid_to_hex(&o->oid), GIT_SHA1_HEXSZ);
> - if (write_in_full(cmd.in, namebuf, 41) < 0)
> - return 0;
> + if (write_in_full(cmd->in, namebuf, 41) < 0)
> + return -1;
> }
> - close(cmd.in);
> + close(cmd->in);
>
> sigchain_pop(SIGPIPE);
Not a new issue, but does it matter that there are early returns
before sigchain_pop()?
> + return 0;
> +}
> +
> +static int check_unreachable(struct object_array *src)
> +{
> + struct child_process cmd = CHILD_PROCESS_INIT;
> + int i, ret = do_reachable_revlist(&cmd, src);
> + char buf[1];
> +
> + if (ret < 0)
> + return 0;
Nit: It might be a bit easier to see what this conditional is checking
if it is closer to the code which sets 'ret'. Perhaps either:
char buf[1];
int i, ret = ...;
if (ret < 0)
or:
char buf[1];
int i, ret;
ret = ...;
if (ret < 0)
> /*
> * The commits out of the rev-list are not ancestors of
> * our ref.
> */
> - i = read_in_full(cmd.out, namebuf, 1);
> + i = read_in_full(cmd.out, buf, 1);
s/1/sizeof(buf)/ perhaps or does that make the intent less clear?
> if (i)
> return 0;
> close(cmd.out);
--
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
