Re: [PATCH] checkout: Don't crash when switching away from an invalid branch.

From: Alexandre Julliard <julliard@winehq.org>
Date: Nov 7, 2008, 22:28 UTC
Message-ID: <87od0r9nnj.fsf@wine.dyndns.org>
In-Reply-To: <alpine.DEB.1.00.0811071903300.30769@pacific.mpi-cbg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 13 quoted lines

>> diff --git a/builtin-checkout.c b/builtin-checkout.c
>> index 57b94d2..7c1b8cd 100644
>> --- a/builtin-checkout.c
>> +++ b/builtin-checkout.c
>> @@ -47,7 +47,8 @@ static int post_checkout_hook(struct commit *old, struct commit *new,
>>  
>>  	memset(&proc, 0, sizeof(proc));
>>  	argv[0] = name;
>> -	argv[1] = xstrdup(sha1_to_hex(old->object.sha1));
>> +	argv[1] = old ? xstrdup(sha1_to_hex(old->object.sha1))
>> +		      : "0000000000000000000000000000000000000000";
>
> I guess you want to use the variable null_sha1 here.

I could, though it seemed to me a bit silly to format and strdup a string that is a known constant. But I'm happy to change it if needed.

Show 18 quoted lines

>> @@ -492,10 +493,13 @@ static void update_refs_for_switch(struct checkout_opts *opts,
>>  	}
>>  
>>  	old_desc = old->name;
>> -	if (!old_desc)
>> +	if (!old_desc && old->commit)
>>  		old_desc = sha1_to_hex(old->commit->object.sha1);
>> -	strbuf_addf(&msg, "checkout: moving from %s to %s",
>> -		    old_desc, new->name);
>> +	if (old_desc)
>> +		strbuf_addf(&msg, "checkout: moving from %s to %s",
>> +			    old_desc, new->name);
>> +	else
>> +		strbuf_addf(&msg, "checkout: moving to %s", new->name);
>
> Why not
> 		old_desc ? old_desc : "(invalid)"
> ?

IMO it looks more friendly to not display a branch that doesn't exist, rather than printing something like (invalid) or (null).

-- 
Alexandre Julliard
julliard@winehq.org