Heiko Voigt <hvoigt@xxxxxxxxxx> writes:
> diff --git a/connect.c b/connect.c
> index 912cdde..19e73d5 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -56,6 +56,8 @@ struct ref **get_remote_heads(int in, struct ref **list,
> unsigned int flags,
> struct extra_have_objects *extra_have)
> {
> + int got_at_least_one_head = 0;
> +
> *list = NULL;
> for (;;) {
> struct ref *ref;
> @@ -64,7 +66,14 @@ struct ref **get_remote_heads(int in, struct ref **list,
> char *name;
> int len, name_len;
>
> - len = packet_read_line(in, buffer, sizeof(buffer));
> + len = packet_read_line(in, buffer, sizeof(buffer), 1);
> + if (len < 0) {
> + if (got_at_least_one_head)
> + die("The remote end hung up unexpectedly");
> + else
> + die("Could not read remote heads");
> + }
I do not think it is particularly interesting to know we have (or
haven't) read one packet before we got an error. It would be an
improvement if the message lets the user know at what stage of the
exchange the remote threw you a garbage, but using the same "The
remote end hung up unexpectedly" as all the other packet_read_line()
errors show makes it less useful.
How about getting rid of the new boolean variable and say
len = packet_read(in, buffer, sizeof(buffer));
if (len < 0)
die("The remote end hung up upon initial contact");
or something?
> if (!len)
> break;
> if (buffer[len-1] == '\n')
> @@ -95,6 +104,7 @@ struct ref **get_remote_heads(int in, struct ref **list,
> hashcpy(ref->old_sha1, old_sha1);
> *list = ref;
> list = &ref->next;
> + got_at_least_one_head = 1;
> }
> return list;
> }
It seems that all callers other than this one after this patch
behave identically as before like this patch. It would be far more
preferable to introduce a new function that does not die on errors
(including but not necessarily limited to short read situation you
are interested in this patch), and update this caller that wants to
handle these error cases to call that new function. Perhaps
len = packet_read(in, buffer, sizeof(buffer));
that returns negative error numbers when it sees an error, with
#define PKTREAD_UNKNOWN_ERROR (-1)
#define PKTREAD_SHORT_READ (-2)
...
and then over time we should consider converting remaining callers
of packet_read_line() to packet_read().
--
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