Re: [PATCH v2] Submodule merge support

From: Shawn O. Pearce <spearce@spearce.org>
Date: May 21, 2007, 06:20 UTC
Message-ID: <20070521062005.GK3141@spearce.org>
In-Reply-To: <20070520154227.GG5412@admingilde.org>

Show 14 quoted lines

> @@ -574,6 +575,21 @@ static void update_file_flags(const unsigned char *sha,
>  		void *buf;
>  		unsigned long size;
>  
> +		if (S_ISDIRLNK(mode)) {
> +			/* defer dirlinks to another process, don't try to */
> +			/* read the object "sha" here */
> +			const char *dirlink_checkout[] = {
> +				"dirlink-checkout", path, sha1_to_hex(sha), NULL
> +			};
> +			struct child_process cmd = {
> +				.argv = dirlink_checkout,
> +				.git_cmd = 1,
> +			};

My Solaris 9 system cannot compile this syntax, even though it is a clean way to initalize the child_process. That's why I've always used something more like:

	struct child_process cmd;
	memset(&cmd, 0, sizeof(cmd));
	cmd.argv = dirlink_checkout;
	cmd.git_cmd = 1;

and actually that raises another point, does the compiler 0 fill the stack-allocated struct that is initalized like you write, or does it avoid filling the other fields that aren't mentioned in the initialization?

> +	status = read(cmd.out, hex, sizeof(hex));
> +	if (status != 40) return status;

OK, this is probably just never trusting the OS, but shouldn't that read be wrapped up in a loop, like our read_in_full? We want 40 bytes here, and expect it, and the read call is allowed to return as few as 1 byte....

-- 
Shawn.