Re: [PATCH v2] commit: accept tag objects in HEAD/MERGE_HEAD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2011/8/18 Junio C Hamano <gitster@xxxxxxxxx>:
> By the way, what happens when you try to merge when HEAD points at a tag
> that points at a commit? Would we end up creating a commit that points at
> a bogus parent?

I guess I'd segfault the same way commit does. It does
lookup_commit(head) without checking the result.

>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index 2088b6b..f327595 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -1387,6 +1387,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>>       unsigned char commit_sha1[20];
>>       struct ref_lock *ref_lock;
>>       struct commit_list *parents = NULL, **pptr = &parents;
>> +     struct commit *commit;
>>       struct stat statbuf;
>>       int allow_fast_forward = 1;
>>       struct wt_status s;
>
> Here, you are being inconsistent with your own argument you made in your
> previous message "later somebody may forget to update the former while
> updating the latter" when I suggested to separate the two logically
> separate operations (grab the head_commit object when necessary, and
> decide how the commit is made). By hoisting of the scope of "commit", you
> made the variable undefined when dealing with the initial_commit, exposing
> the code to the same risk that somebody may try to use "commit" variable
> after the if/elseif/... cascade, where it may or may not be defined.

Fair enough.

>> @@ -1423,12 +1424,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>>                       reflog_msg = "commit (initial)";
>>       } else if (amend) {
>>               struct commit_list *c;
>> -             struct commit *commit;
>>
>>               if (!reflog_msg)
>>                       reflog_msg = "commit (amend)";
>> -             commit = lookup_commit(head_sha1);
>> -             if (!commit || parse_commit(commit))
>> +             commit = lookup_expect_commit(head_sha1, "HEAD");
>> +             if (parse_commit(commit))
>>                       die(_("could not parse HEAD commit"));
>
> Is this still necessary? I think your lookup_expect_commit() already
> checks this condition and barfs.

It is. lookup_expect_commit() does not parse commit content, only
makes sure it's a commit object.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]