From: Shawn O. Pearce <spearce@spearce.org>
Date: Mon, 21 May 2007 06:20:05 GMT
Subject: Re: [PATCH v2] Submodule merge support
Message-ID: <20070521062005.GK3141@spearce.org>
In-Reply-To: <20070520154227.GG5412@admingilde.org>

> @@ -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.