Re: [PATCH v2] Submodule merge support

From: Junio C Hamano <junkio@cox.net>
Date: May 21, 2007, 07:54 UTC
Message-ID: <7vabvyfw7n.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20070521073253.GU5412@admingilde.org>

Martin Waitz <tali@admingilde.org> writes:

Show 16 quoted lines

>> > +		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.
>
> any special thing it does not like in the above code or does it just
> not support structs that are initialized that way?

Portability rules:

 - We do not do C99 initializers;
 - We do not do decl-after-statement;

Readability rules:

 - We always write NULL, not 0, for a NULL pointer.

There may be a handful more unwritten rules we use.

Show 9 quoted lines

>> > +	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....
>
> right.

I think we have read-in-full or something like that for this exact purpose.