From: Alexandre Julliard Date: Fri, 07 Nov 2008 22:28:16 GMT Subject: Re: [PATCH] checkout: Don't crash when switching away from an invalid branch. Message-ID: <87od0r9nnj.fsf@wine.dyndns.org> In-Reply-To: Johannes Schindelin writes: >> 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. >> @@ -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