[PATCH?] Fix 'git show' on signed tag of signed tag of commit

[PATCH?] Fix 'git show' on signed tag of signed tag of commit

[Thomas Rast]

The cmd_show loop resolves tags by showing them, then pointing the
object to the 'tagged' member.  However, this object is not fully
initialized; it only contains the SHA1.  (This resulted in a segfault
if there were two levels of tags.)  We apply parse_object to get a
full object.

Noticed by Kalle Olavi Niemitalo on IRC.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

I'm not 100% sure this is the right fix; I just had a quick stab at
the code, made a fix and tested it.  It probably leaks memory too, not
sure if this is a problem.

- Thomas


 builtin-log.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 9817d6f..9979e37 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -360,7 +360,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 					t->tag,
 					diff_get_color_opt(&rev.diffopt, DIFF_RESET));
 			ret = show_object(o->sha1, 1, &rev);
-			objects[i].item = (struct object *)t->tagged;
+			objects[i].item = parse_object(t->tagged->sha1);
 			i--;
 			break;
 		}
-- 
1.5.6.1.246.g5fc88.dirty

Re: [PATCH?] Fix 'git show' on signed tag of signed tag of commit

[Johannes Schindelin]

Hi,

On Tue, 1 Jul 2008, Thomas Rast wrote:

> The cmd_show loop resolves tags by showing them, then pointing the
> object to the 'tagged' member.  However, this object is not fully
> initialized; it only contains the SHA1.  (This resulted in a segfault
> if there were two levels of tags.)  We apply parse_object to get a
> full object.
> 
> Noticed by Kalle Olavi Niemitalo on IRC.
> 
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
> 
> I'm not 100% sure this is the right fix; I just had a quick stab at the 
> code, made a fix and tested it.  It probably leaks memory too, not sure 
> if this is a problem.

AFAICT this is the correct way to solve the problem.

We do not "leak" memory here, since we just assume objects to be 
singletons (they are immutable, after all).

FWIW I had a "release_all_objects()" function in my personal fork for a 
long time, which was quite painful to write.  But maintaining the "move 
all builtins into libgit.a and avoid fork()&&exec()ing them" patch was the 
only user, and since it became way too painful to maintain out of the 
tree, I dropped both.

Ciao,
Dscho

Re: [PATCH?] Fix 'git show' on signed tag of signed tag of commit

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> The cmd_show loop resolves tags by showing them, then pointing the
> object to the 'tagged' member.  However, this object is not fully
> initialized; it only contains the SHA1.  (This resulted in a segfault
> if there were two levels of tags.)  We apply parse_object to get a
> full object.
>
> Noticed by Kalle Olavi Niemitalo on IRC.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>
> I'm not 100% sure this is the right fix; I just had a quick stab at
> the code, made a fix and tested it.  It probably leaks memory too, not
> sure if this is a problem.

The code looks good.  It should not have been accessing the object without
making sure it has been parsed.

And it's not a leak.  Once you instantiate "struct object" derived class,
we hold onto it (at least to its skeleton).

Thanks.