Re: Segfault in git reflog

[Duy Nguyen <pclouds@gmail.com>]

On Wed, Dec 30, 2015 at 01:28:06PM +0100, Dennis Kaarsemaker wrote:
> On wo, 2015-12-30 at 18:28 +0700, Duy Nguyen wrote:
> > On Wed, Dec 30, 2015 at 6:26 PM, Duy Nguyen <pclouds@gmail.com>
> > wrote:
> > > On Wed, Dec 30, 2015 at 6:17 PM, Dennis Kaarsemaker
> > > <dennis@kaarsemaker.net> wrote:
> > >> On wo, 2015-12-30 at 10:24 +0100, Dennis Kaarsemaker wrote:
> > >>> spirit:~/code/git (master)$ cat .git/logs/HEAD
> > >>> 2635c2b8bfc9aec07b7f023d8e3b3d02df715344
> > 54bc41416c5d3ecb978acb0df80d57aa3e54494c Dennis Kaarsemaker <
> > dennis@kaarsemaker.net> 1446765642 +0100
> > >>> 74c855f87d25a5b5c12d0485ec77c785a1c734c5
> > 54bc41416c5d3ecb978acb0df80d57aa3e54494c Dennis Kaarsemaker <
> > dennis@kaarsemaker.net> 1446765951 +0100  checkout: moving from
> > 3c3d3f629a6176b401ebec455c5dd59ed1b5f910 to master
> > 
> > Ah... I came from a different angle and did not realize the tag sha1
> > is from your reflog. So yeah maybe reflog parsing code should check
> > object type first, don't assume it's a commit!
> 
> Something like this perhaps?
> 
> diff --git a/reflog-walk.c b/reflog-walk.c
> index 85b8a54..cd538dd 100644
> --- a/reflog-walk.c
> +++ b/reflog-walk.c
> @@ -25,6 +25,14 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
>  {
>         struct complete_reflogs *array = cb_data;
>         struct reflog_info *item;
> +       struct object *obj;
> +
> +       obj = parse_object(osha1);
> +       if(obj && obj->type != OBJ_COMMIT)
> +               die(_("Broken reflog, %s is a %s, not a commit"), sha1_to_hex(obj->oid.hash), typename(obj->type));
> +       obj = parse_object(nsha1);
> +       if(obj && obj->type != OBJ_COMMIT)
> +               die(_("Broken reflog, %s is a %s, not a commit"), sha1_to_hex(obj->oid.hash), typename(obj->type));
>  
>         ALLOC_GROW(array->items, array->nr + 1, array->alloc);
>         item = array->items + array->nr;
> 
> That gives me:
> fatal: Broken reflog, 74c855f87d25a5b5c12d0485ec77c785a1c734c5 is a tag, not a commit

I would go with something like this. The typecasting to "struct commit
*" is the bug because parse_object() can return any object type. With
this I got

error: object 74c855f87d25a5b5c12d0485ec77c785a1c734c5 is a tag, not a commit

diff --git a/reflog-walk.c b/reflog-walk.c
index 85b8a54..09d18fa 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -236,8 +236,9 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 	reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
 	info->last_commit_reflog = commit_reflog;
 	commit_reflog->recno--;
-	commit_info->commit = (struct commit *)parse_object(reflog->osha1);
-	if (!commit_info->commit) {
+	commit_info->commit = lookup_commit(reflog->osha1);
+	if (!commit_info->commit ||
+	    parse_commit(commit_info->commit)) {
 		commit->parents = NULL;
 		return;
 	}
--
Duy