Hi,

On Fri, 8 Feb 2008, Jan Holesovsky wrote:

quoted text
> +static void send_want(int fd[2], const char *remote, int full_info)
> +{
> +	if (full_info)
> +		packet_write(fd[1], "want %s%s%s%s%s%s%s%s%s\n",
> +				remote,
> +				(multi_ack ? " multi_ack" : ""),
> +				(use_sideband == 2 ? " side-band-64k" : ""),
> +				(use_sideband == 1 ? " side-band" : ""),
> +				(args.use_thin_pack ? " thin-pack" : ""),
> +				(args.no_progress ? " no-progress" : ""),
> +				(args.commits_only ? " commits-only" : ""),
> +				(args.exact_objects ? " exact-objects" : ""),
> +				" ofs-delta");
> +	else
> +		packet_write(fd[1], "want %s\n", remote);
> +}

You might want to make the full_info static, and only send the options the 
first time.

quoted text
> @@ -523,11 +541,15 @@ static int get_pack(int xd[2], char **pack_lockfile)
>  				strcpy(keep_arg + s, "localhost");
>  			*av++ = keep_arg;
>  		}
> +		if (args.exact_objects)
> +			*av++ = "--ignore-remote-alternates";
>  	}
>  	else {
>  		*av++ = "unpack-objects";
>  		if (args.quiet)
>  			*av++ = "-q";
> +		if (args.exact_objects)
> +			*av++ = "--ignore-remote-alternates";
>  	}

You can move this outside of the if() instead of repeating yourself...

quoted text
> @@ -556,6 +578,7 @@ static struct ref *do_fetch_pack(int fd[2],
>  	unsigned char sha1[20];
>  
>  	get_remote_heads(fd[0], &ref, 0, NULL, 0);
> +
>  	if (is_repository_shallow() && !server_supports("shallow"))
>  		die("Server does not support shallow clients");
>  	if (server_supports("multi_ack")) {

Not strictly necessary, right? ;-)

quoted text
> @@ -647,12 +686,72 @@ static void fetch_pack_setup(void)
>  	did_setup = 1;
>  }
>  
> +static void read_from_stdin(int *num, char ***records)
> +{
> +	char buffer[4096];
> +	size_t records_num, leftover;
> +	ssize_t ret;
> +
> +	*num = 0;
> +	leftover = 0;
> +
> +	records_num = 4096;
> +	(*records) = xmalloc(records_num * sizeof(char *));
> +
> +	do {
> +		char *p, *last;
> +
> +		ret = xread(0 /*stdin*/, buffer + leftover,
> +				sizeof(buffer) - leftover);
> +		if (ret < 0)
> +			die("read error on input: %s", strerror(errno));
> +
> +		last = buffer;
> +		for (p = buffer; p < buffer + leftover + ret; p++)
> +			if ((!*p || *p == '\n') && (p != last)) {
> +				if (*num >= records_num) {
> +					records_num *= 2;
> +					(*records) = xrealloc(*records,
> +							      records_num * sizeof(char*));
> +				}
> +
> +				if (p - last > 0) {
> +					(*records)[*num] =
> +						strndup(last, p - last);
> +					(*num)++;
> +				}
> +				last = p + 1;
> +			}
> +
> +		leftover = p - last;
> +		if (leftover >= sizeof(buffer))
> +			die("input line too long");
> +		if (leftover < 0)
> +			leftover = 0;
> +
> +		memmove(buffer, last, leftover);
> +	} while (ret > 0);
> +
> +	if (leftover) {
> +		if (*num >= records_num) {
> +			records_num *= 2;
> +			(*records) = xrealloc(*records,
> +					      records_num * sizeof(char*));
> +		}
> +
> +		(*records)[*num] = strndup(buffer, leftover);
> +		(*num)++;
> +	}
> +}
> +

This chunk could use ALLOC_GROW() quite nicely (would make it more 
readable, and avoid errors).  Also, I'd use alloc_nr() instead of the 
doubling.

quoted text
>  int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
>  {
>  	int i, ret, nr_heads;
>  	struct ref *ref;
>  	char *dest = NULL, **heads;
> +	int from_stdin;
>  
> +	from_stdin = 0;

You can initialise it to 0 right away...

Unfortunately, I have to go now... so I will review the rest 
(from builtin-fetch.c on) later.

It's great seeing that you work on this!

Thanks,
Dscho
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html