Jeff King <peff@xxxxxxxx> writes:
> diff --git a/builtin-send-pack.c b/builtin-send-pack.c
> index c7d07aa..bcf7143 100644
> --- a/builtin-send-pack.c
> +++ b/builtin-send-pack.c
> @@ -146,19 +146,43 @@ static void get_local_heads(void)
> for_each_ref(one_local_ref, NULL);
> }
>
> -static int receive_status(int in)
> +static struct ref *set_ref_error(struct ref *refs, const char *line)
> {
> + struct ref *ref;
> +
> + for (ref = refs; ref; ref = ref->next) {
> + const char *msg;
> + if (prefixcmp(line, ref->name))
> + continue;
> + msg = line + strlen(ref->name);
> + if (*msg++ != ' ')
> + continue;
> + ref->status = REF_STATUS_REMOTE_REJECT;
> + ref->error = xstrdup(msg);
> + ref->error[strlen(ref->error)-1] = '\0';
> + return ref;
> + }
> + return NULL;
> +}
It probably would not matter for sane repositories, but with
thousands of refs, strlen() and prefixcmp() may start to hurt:
struct ref *ref;
int reflen;
const char *msg = strchr(line, ' ');
if (!msg)
return NULL;
reflen = msg - line;
msg++;
for (ref = refs; ref; ref = ref->next) {
if (strncmp(line, ref->name, reflen) || line[reflen] != ' ')
continue;
...
return ref->next;
}
return NULL;
but the "hint" optimization probably make the above
micro-optimization irrelevant.
> +/* a return value of -1 indicates that an error occurred,
> + * but we were able to set individual ref errors. A return
> + * value of -2 means we couldn't even get that far. */
It is preferred to have a multi-line comment like this:
/*
* A return value of -1 ...
* ...
* ... couldn't even get that far.
*/
> +static int receive_status(int in, struct ref *refs)
> ...
> + hint = NULL;
> while (1) {
> len = packet_read_line(in, line, sizeof(line));
> if (!len)
> @@ -171,7 +195,10 @@ static int receive_status(int in)
> }
> if (!memcmp(line, "ok", 2))
> continue;
> - fputs(line, stderr);
> + if (hint)
> + hint = set_ref_error(hint, line + 3);
> + if (!hint)
> + hint = set_ref_error(refs, line + 3);
Clever... taking advantage of the order receive-pack reports to
optimize.
Before receive_status() is called, can the refs already have the
error status and string set?
> @@ -429,12 +463,15 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
> }
> close(out);
>
> - print_push_status(dest, remote_refs);
> -
> if (expect_status_report) {
> - if (receive_status(in))
> + ret = receive_status(in, remote_refs);
> + if (ret == -2)
> return -1;
Hmm. When we did not receive status, we cannot tell what
succeeded or failed, but what we _can_ tell the user is which
refs we attempted to push. I wonder if robbing that information
from the user with this "return -1" is a good idea. Perhaps we
would instead want to set the status of all the refs to error
and call print_push_status() anyway? I dunno.
> }
> + else
> + ret = 0;
> +
> + print_push_status(dest, remote_refs);
>
> if (!args.dry_run && remote) {
> for (ref = remote_refs; ref; ref = ref->next)
-
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
