threads / patch / 14242

patchFix 'git show' on signed tag of signed tag of commit

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

## tl;dr

3 messages between Jul 1, 2008 and Jul 1, 2008. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Thomas Rast· Jul 1, 2008, 09:47 UTC · lore

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(-)

Show changes to builtin-log.c +1 −1

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

Johannes Schindelin· Jul 1, 2008, 16:39 UTC · re: Thomas Rast · lore

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

Hi,

On Tue, 1 Jul 2008, Thomas Rast wrote:

Show 14 quoted lines

> 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

Junio C Hamano· Jul 1, 2008, 23:55 UTC · re: Thomas Rast · lore

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

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

Show 14 quoted lines

> 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.