bug found (Re: git-fast-export SIGSEGV on solaris + backtrace)

Hello,

 From the gdb trace:

(gdb) p deco
$1 = (struct object_decoration *) 0x193150
(gdb) p idnums
$3 = {name = 0x0, size = 1500, nr = 7, hash = 0x190270}

It was 'off-by-one' error.
0x190270 + (1500 * 8) = 0x193150.

And the code says it:
         for (i = 0; i < idnums.size; ++i) {
                 deco++;
                 if (deco && deco->base && deco->base->type == 1) {

The 'deco' should be post-incremented? or
Checking code should be  (i < idnums.size - 1)?

Regards,
namsh

And very minor cleanup:

diff --git a/decorate.c b/decorate.c
index 23f6b00..d8b428c 100644
--- a/decorate.c
+++ b/decorate.c

@@ -36,7 +36,7 @@ static void *insert_decoration(struct decoration *n, 

struct o>
  static void grow_decoration(struct decoration *n)
  {
         int i;
-       int old_size = n->size;
+       int old_size;
         struct object_decoration *old_hash;

         old_size = n->size;

SungHyun Nam wrote:

Hello,

Because recent GIT test failed on t9301-fast-export.sh, I traced
it. And I found git-fast-export killed by SIGSEGV. I include a
gdb backtrace below.
If you want to me to check other things, please let me know.

Regards,
namsh

[marks] ~/srcs/git/t/trash directory[66]$ gdb ../../git-fast-export
GNU gdb 6.0
Copyright 2003 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you 
are
welcome to change it and/or distribute copies of it under certain 
conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "sparc-sun-solaris2.9"...
(gdb) r --export-marks=tmp-marks HEAD
Starting program: /flyvo2/home/namsh/srcs/git/git-fast-export 
--export-marks=tmp-marks HEAD
blob
mark :1
data 8
Wohlauf

reset refs/heads/marks
commit refs/heads/marks
mark :2
author A U Thor [off-list ref] 1112911993 -0700
committer C O Mitter [off-list ref] 1112911993 -0700
data 8
initial
M 100644 :1 file

blob
mark :3
data 9
die Luft

blob
mark :4
data 12
geht frisch

commit refs/heads/marks
mark :5
author A U Thor [off-list ref] 1112912053 -0700
committer C O Mitter [off-list ref] 1112912053 -0700
data 7
second
from :2
M 100644 :3 file
M 100644 :4 file2

blob
mark :6
data 4
und

commit refs/heads/marks
mark :7
author A U Thor [off-list ref] 1112912113 -0700
committer C O Mitter [off-list ref] 1112912113 -0700
data 6
third
from :5
M 100644 :6 file2


Program received signal SIGSEGV, Segmentation fault.
0x00043024 in export_marks (file=0xffbff0df "tmp-marks")
    at builtin-fast-export.c:384
384                     if (deco && deco->base && deco->base->type == 1) {
(gdb) p deco
$1 = (struct object_decoration *) 0x193150
(gdb) p deco->base
$2 = (struct object *) 0x2009
(gdb) p *(struct object_decoration *) 0x193150
$5 = {base = 0x2009, decoration = 0x0}
(gdb) p idnums
$3 = {name = 0x0, size = 1500, nr = 7, hash = 0x190270}
(gdb) p *(struct object_decoration *) 0x190270
$6 = {base = 0x0, decoration = 0x0}
(gdb) p *(struct object_decoration *) 0x190280
$9 = {base = 0x0, decoration = 0x0}
(gdb) p *(struct object_decoration *) 0x190290
$10 = {base = 0x0, decoration = 0x0}
(gdb) p *(struct object_decoration *) 0x1902a0
$11 = {base = 0x0, decoration = 0x0}
(gdb) p *(struct object_decoration *) 0x1902b0
$12 = {base = 0x0, decoration = 0x0}
(gdb) p *(struct object_decoration *) 0x1902c0
$13 = {base = 0x0, decoration = 0x0}
(gdb) p *(struct object_decoration *) 0x193140
$14 = {base = 0x0, decoration = 0x0}
(gdb) p *(struct object_decoration *) 0x193160
$15 = {base = 0x35323962, decoration = 0x63613534}
(gdb)

SungHyun Nam [off-list ref] writes:

And the code says it:
        for (i = 0; i < idnums.size; ++i) {
                deco++;
                if (deco && deco->base && deco->base->type == 1) {

The 'deco' should be post-incremented? or
Checking code should be  (i < idnums.size - 1)?

The variable "deco" is a pointer that walks over a hashtable from its
offset 0 to its end, so it can never be NULL (well, the code increments
before it tests the variable for NULLness, so it is clear that the test is
bogus).

What was I smoking when I applied df6a7ff (builtin-fast-export: Add
importing and exporting of revision marks, 2008-06-11), I have to
wonder...

Thanks for the fix.

Hi,

On Thu, 3 Jul 2008, Junio C Hamano wrote:

What was I smoking when I applied df6a7ff (builtin-fast-export: Add 
importing and exporting of revision marks, 2008-06-11), I have to 
wonder...

FWIW I missed it in my review, too...  Well, Junio, it seems we should 
avoid smoking together for a while.

Ciao,
Dscho