chromatic wrote:
> Here's a patch which nearly gets t/pmc/packfiledirectory.t working fully.  
> I've added set_pmc_keyed() and set_pmc_keyed_str() to the PackfileDirectory 
> PMC.  Unfortunately, this causes crashes when destroying the directory, as it 
> looks like I'm sharing memory in two places.
> 
> This test is a little dodgy as well, because it tries to add a segment which 
> already exists in the directory again with a different name.  I'm not sure 
> that should work, but I couldn't get other approaches to work much better.

Huh?  I don't see it copying an already-existing segment.  It looks to
me like the test creates an entirely new (empty) segment.  But I like
this latter approach better.

The problem was, the new segment structure's "type" and "pf" members
were uninitialized.  The "pf" member was NULL causing the first
segfault.  Once that worked, the "type" member was 0 (which means,
PF_DIR_SEG), making the destructor call the wrong free function, which
resulted in a second segfault.

The attached patch, applied on top of yours, gets the test passing.

Mark
commit f02bd405ef12bab356b10b18e5a7d07930cdaa21
Author: Mark Glines <[email protected]>
Date:   Fri Feb 20 13:24:06 2009 -0800

    Fix things up well enough to pass the test.

diff --git a/src/pmc/packfiledirectory.pmc b/src/pmc/packfiledirectory.pmc
index 6c91ac3..3bd781a 100644
--- a/src/pmc/packfiledirectory.pmc
+++ b/src/pmc/packfiledirectory.pmc
@@ -160,8 +160,8 @@ name already existed, it will be replaced with the new 
segment.
 
 */
     VTABLE void set_pmc_keyed_str(STRING *name, PMC *segment)  {
-        const PackFile_Directory * const pfd = PMC_data_typed(SELF, 
PackFile_Directory *);
-        PackFile_Segment   * const add_seg = PMC_data(segment);
+        PackFile_Directory * const pfd = PMC_data_typed(SELF, 
PackFile_Directory *);
+        PackFile_Segment *add_seg = PMC_data_typed(segment, PackFile_Segment 
*);
 
         const int total = pfd->num_segments;
         int       i;
@@ -171,12 +171,14 @@ name already existed, it will be replaced with the new 
segment.
 
             if (!Parrot_str_compare(interp, name,
                     Parrot_str_new_constant(interp, pfseg->name))) {
+                    add_seg->pf = pfd->base.pf;
                     pfd->segments[i] = add_seg;
                 return;
             }
         }
 
         add_seg->name = Parrot_str_to_cstring(interp, name);
+        add_seg->pf = pfd->base.pf;
         PackFile_add_segment(interp, pfd, add_seg);
     }
 
diff --git a/src/pmc/packfilerawsegment.pmc b/src/pmc/packfilerawsegment.pmc
index 11dcb07..5722e0e 100644
--- a/src/pmc/packfilerawsegment.pmc
+++ b/src/pmc/packfilerawsegment.pmc
@@ -29,6 +29,22 @@ PDD13 for the design spec.
 pmclass PackfileRawSegment extends PackfileSegment {
 
 
+    /*
+
+    =item C<void init()>
+
+    Initialize the structure.  (Create a blank PackFile_Segment object.)
+
+    =cut
+
+    */
+        VTABLE void init() {
+            PackFile_Segment *seg = PackFile_Segment_new(interp, NULL, "", 0);
+            PMC_data(SELF) = seg;
+            seg->type = PF_UNKNOWN_SEG;
+        }
+
+
 /*
 
 =item C<INTVAL elements()>
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev

Reply via email to