delete hierarhical pin sheet bug

change_log.txt

@@ -5,6 +5,22 @@ Started 2007-June-11
 Please add newer entries at the top, list the date and your name with
 email address.
 
+
+2008-Apr-22 UPDATE   Dick Hollenbeck <dick@softplc.com>
+================================================================================
++eeschema
+  * Spent a 1/2 day tracking down two linked list bugs in deleting a
+    DRAW_HIERARCHICAL_PIN_SHEET_STRUCT_TYPE.   I cannot believe in the year
+    2008 we should have to debug a linked list function.  This is stuff I expected
+    to do 20 years ago, not today.  The function
+    void WinEDA_SchematicFrame::DeleteSheetLabel( wxDC* DC,
+              Hierarchical_PIN_Sheet_Struct* SheetLabelToDel ) never worked as
+    far as I can tell.
+    Should switch to boost::ptr_vector ASAP everywhere, and leave linked lists in the 1980's.
+  * Hierarchical_PIN_Sheet_Struct::Hierarchical_PIN_Sheet_Struct() was not
+    setting the m_Parent.
+
+
 2008-Apr-21 UPDATE   Jean-Pierre Charras <jean-pierre.charras@inpg.fr>
 ================================================================================
 +eeschema

common/base_struct.cpp

@@ -161,22 +161,12 @@ std::ostream& operator<<( std::ostream& out, const wxPoint& pt )
  */
 void EDA_BaseStruct::Show( int nestLevel, std::ostream& os )
 {
-    // for now, make it look like XML:
+    // XML output:
     wxString s = GetClass();
 
-    s = s + wxT( " " );
-    NestedSpace( nestLevel, os ) << '<' << s.Lower().mb_str() << ">\n";
-
-    /*
-     * EDA_BaseStruct* kid = m_Son;
-     * for( ; kid;  kid = kid->Pnext )
-     * {
-     * kid->Show( nestLevel+1, os );
-     * }
-     */
-    NestedSpace( nestLevel + 1, os ) << "Need ::Show() override\n";
-
-    NestedSpace( nestLevel, os ) << "</" << s.Lower().mb_str() << ">\n";
+    NestedSpace( nestLevel, os ) << '<' << s.Lower().mb_str() << ">"
+        << " Need ::Show() override for this class "
+        << "</" << s.Lower().mb_str() << ">\n";
 }
 
 
@@ -195,7 +185,6 @@ std::ostream& EDA_BaseStruct::NestedSpace( int nestLevel, std::ostream& os )
     return os;
 }
 
-
 #endif
 
 

eeschema/class_drawsheet.cpp

@@ -631,6 +631,28 @@ bool DrawSheetStruct::ChangeFileName( WinEDA_SchematicFrame* aFrame, const wxStr
 }
 
 
+#if defined(DEBUG)
+void DrawSheetStruct::Show( int nestLevel, std::ostream& os )
+{
+    // XML output:
+    wxString s = GetClass();
+
+    NestedSpace( nestLevel, os ) << '<' << s.Lower().mb_str() << ">"
+        << " sheet_name=\"" << CONV_TO_UTF8( m_SheetName) << '"'
+        << ">\n";
+
+    // show all the pins, and check the linked list integrity
+    Hierarchical_PIN_Sheet_Struct* label;
+    for( label = m_Label;   label;   label=label->Next() )
+    {
+        label->Show( nestLevel+1, os );
+    }
+
+    NestedSpace( nestLevel, os ) << "</" << s.Lower().mb_str() << ">\n"
+        << std::flush;
+}
+#endif
+
 
 /**********************************************/
 /* class to handle a series of sheets *********/
@@ -815,3 +837,4 @@ bool DrawSheetPath::operator!=( const DrawSheetPath& d1 )
 
     return false;
 }
+

eeschema/class_drawsheet.h

@@ -29,6 +29,7 @@ public:
                                    const wxString& text = wxEmptyString );
 
     ~Hierarchical_PIN_Sheet_Struct() { }
+
     virtual wxString GetClass() const
     {
         return wxT( "Hierarchical_PIN_Sheet_Struct" );
@@ -37,8 +38,7 @@ public:
 
     Hierarchical_PIN_Sheet_Struct*  GenCopy();
 
-    Hierarchical_PIN_Sheet_Struct* Next()
-    { return (Hierarchical_PIN_Sheet_Struct*) Pnext; }
+    Hierarchical_PIN_Sheet_Struct* Next() { return (Hierarchical_PIN_Sheet_Struct*) Pnext; }
 
     void        Place( WinEDA_SchematicFrame* frame, wxDC* DC );
     void        Draw( WinEDA_DrawPanel* panel, wxDC* DC, const wxPoint& offset,
@@ -51,6 +51,11 @@ public:
      * @return bool - true if success writing else false.
      */
     bool        Save( FILE* aFile ) const;
+
+#if defined(DEBUG)
+    // comment inherited by Doxygen from Base_Struct
+    void        Show( int nestLevel, std::ostream& os );
+#endif
 };
 
 
@@ -120,6 +125,11 @@ public:
     //void 		RemoveSheet(DrawSheetStruct* sheet);
     //to remove a sheet, just delete it
     //-- the destructor should take care of everything else.
+
+#if defined(DEBUG)
+    // comment inherited by Doxygen from Base_Struct
+    void                Show( int nestLevel, std::ostream& os );
+#endif
 };
 
 

eeschema/class_hierarchical_PIN_sheet.cpp

@@ -36,10 +36,12 @@
 /*******************************************************************/
 Hierarchical_PIN_Sheet_Struct::Hierarchical_PIN_Sheet_Struct( DrawSheetStruct* parent,
                                             const wxPoint& pos, const wxString& text ) :
-    SCH_ITEM( NULL, DRAW_HIERARCHICAL_PIN_SHEET_STRUCT_TYPE ),
+    SCH_ITEM( parent, DRAW_HIERARCHICAL_PIN_SHEET_STRUCT_TYPE ),
     EDA_TextStruct( text )
 /*******************************************************************/
 {
+    wxASSERT( parent );
+    wxASSERT( Pnext == NULL );
     m_Layer      = LAYER_SHEETLABEL;
     m_Pos        = pos;
     m_Edge       = 0;
@@ -199,3 +201,19 @@ bool Hierarchical_PIN_Sheet_Struct::Save( FILE* aFile ) const
     return true;
 }
 
+#if defined(DEBUG)
+void Hierarchical_PIN_Sheet_Struct::Show( int nestLevel, std::ostream& os )
+{
+    // XML output:
+    wxString s = GetClass();
+
+    NestedSpace( nestLevel, os ) << '<' << s.Lower().mb_str() << ">"
+        << " pin_name=\"" << CONV_TO_UTF8( m_Text ) << '"'
+        << "/>\n"
+        << std::flush;
+
+//    NestedSpace( nestLevel, os ) << "</" << s.Lower().mb_str() << ">\n";
+
+}
+
+#endif

eeschema/class_schematic_items.cpp

@@ -154,6 +154,20 @@ EDA_Rect DrawJunctionStruct::GetBoundingBox()
 };
 
 
+#if defined(DEBUG)
+void DrawJunctionStruct::Show( int nestLevel, std::ostream& os )
+{
+    // XML output:
+    wxString s = GetClass();
+
+    NestedSpace( nestLevel, os ) << '<' << s.Lower().mb_str()
+        << m_Pos
+        << "/>\n";
+}
+
+#endif
+
+
 /*****************************/
 /* class DrawNoConnectStruct */
 /*****************************/
@@ -245,8 +259,6 @@ void DrawMarkerStruct::Show( int nestLevel, std::ostream& os )
     NestedSpace( nestLevel, os ) << '<' << GetClass().Lower().mb_str() << m_Pos
                                  << "/>\n";
 }
-
-
 #endif
 
 /**

eeschema/class_text-label.cpp

@@ -196,6 +196,24 @@ bool SCH_TEXT::Save( FILE* aFile ) const
 }
 
 
+#if defined(DEBUG)
+
+void SCH_TEXT::Show( int nestLevel, std::ostream& os )
+{
+    // XML output:
+    wxString s = GetClass();
+
+    NestedSpace( nestLevel, os ) << '<' << s.Lower().mb_str()
+        << " layer=\"" << m_Layer << '"'
+        << " shape=\"" << m_Shape << '"'
+        << " dangling=\"" << m_IsDangling << '"'
+        << '>'
+        << CONV_TO_UTF8( m_Text )
+        << "</" << s.Lower().mb_str() << ">\n";
+}
+
+#endif
+
 /****************************************************************************/
 SCH_LABEL::SCH_LABEL( const wxPoint& pos, const wxString& text ) :
     SCH_TEXT( pos, text, TYPE_SCH_LABEL )

eeschema/class_text-label.h

@@ -112,6 +112,11 @@ public:
      */
     bool    Save( FILE* aFile ) const;
 
+#if defined(DEBUG)
+    void Show( int nestLevel, std::ostream& os );
+
+#endif
+
 };
 
 

eeschema/dangling_ends.cpp

@@ -62,9 +62,12 @@ bool SegmentIntersect( int Sx1, int Sy1, int Sx2, int Sy2,
 
     if( Sx1 == Sx2 )          /* Line S is vertical. */
     {
-        Symin = MIN( Sy1, Sy2 ); Symax = MAX( Sy1, Sy2 );
+        Symin = MIN( Sy1, Sy2 );
+        Symax = MAX( Sy1, Sy2 );
+
         if( Px1 != Sx1 )
             return FALSE;
+
         if( Py1 >= Symin  &&  Py1 <= Symax )
             return TRUE;
         else
@@ -72,9 +75,12 @@ bool SegmentIntersect( int Sx1, int Sy1, int Sx2, int Sy2,
     }
     else if( Sy1 == Sy2 )    /* Line S is horizontal. */
     {
-        Sxmin = MIN( Sx1, Sx2 ); Sxmax = MAX( Sx1, Sx2 );
+        Sxmin = MIN( Sx1, Sx2 );
+        Sxmax = MAX( Sx1, Sx2 );
+
         if( Py1 != Sy1 )
             return FALSE;
+
         if( Px1 >= Sxmin  &&  Px1 <= Sxmax )
             return TRUE;
         else
@@ -92,35 +98,34 @@ void WinEDA_SchematicFrame::TestDanglingEnds( SCH_ITEM* DrawList, wxDC* DC )
 /* Met a jour les membres m_Dangling des wires, bus, labels
  */
 {
-    EDA_BaseStruct*          DrawItem;
-    const DanglingEndHandle* DanglingItem, * nextitem;
-
     if( ItemList )
+    {
+        const DanglingEndHandle* DanglingItem;
+        const DanglingEndHandle* nextitem;
+
         for( DanglingItem = ItemList; DanglingItem != NULL; DanglingItem = nextitem )
         {
             nextitem = DanglingItem->m_Pnext;
             SAFE_DELETE( DanglingItem );
         }
+    }
 
     ItemList = RebuildEndList( DrawList );
 
     // Controle des elements
-    for( DrawItem = DrawList; DrawItem != NULL; DrawItem = DrawItem->Next() )
+    for( SCH_ITEM* item = DrawList;  item;  item = item->Next() )
     {
-        switch( DrawItem->Type() )
+        switch( item->Type() )
         {
         case TYPE_SCH_GLOBALLABEL:
         case TYPE_SCH_HIERLABEL:
         case TYPE_SCH_LABEL:
-                #undef STRUCT
-                #define STRUCT ( (SCH_LABEL*) DrawItem )
-            TestLabelForDangling( STRUCT, this, DC );
-            break;
+            TestLabelForDangling( (SCH_LABEL*) item, this, DC );
             break;
 
         case DRAW_SEGMENT_STRUCT_TYPE:
             #undef STRUCT
-                #define STRUCT ( (EDA_DrawLineStruct*) DrawItem )
+            #define STRUCT ( (EDA_DrawLineStruct*) item )
             if( STRUCT->GetLayer() == LAYER_WIRE )
             {
                 TestWireForDangling( STRUCT, this, DC );
@@ -433,25 +438,25 @@ DanglingEndHandle* RebuildEndList( EDA_BaseStruct* DrawList )
 
         case DRAW_SHEET_STRUCT_TYPE:
             {
-            #undef STRUCT
-            #define STRUCT ( (DrawSheetStruct*) DrawItem )
-            Hierarchical_PIN_Sheet_Struct* pinsheet = STRUCT->m_Label;
-            while( pinsheet )
+                Hierarchical_PIN_Sheet_Struct* pinsheet;
+                for( pinsheet = ((DrawSheetStruct*)DrawItem)->m_Label;  pinsheet;  pinsheet = pinsheet->Next() )
                 {
+                    wxASSERT( pinsheet->Type() == DRAW_HIERARCHICAL_PIN_SHEET_STRUCT_TYPE );
+
                     item = new DanglingEndHandle( SHEET_LABEL_END );
 
                     item->m_Item = pinsheet;
                     item->m_Pos  = pinsheet->m_Pos;
+
                     if( lastitem )
                         lastitem->m_Pnext = item;
                     else
                         StartList = item;
+
                     lastitem = item;
-                pinsheet = (Hierarchical_PIN_Sheet_Struct*) pinsheet->Pnext;
                 }
-
-            break;
             }
+            break;
 
         default:
             ;

eeschema/load_one_schematic_file.cpp

@@ -473,7 +473,7 @@ bool WinEDA_SchematicFrame::LoadOneEEFile( SCH_SCREEN* screen, const wxString& F
 
     screen->EEDrawList = Phead;
 
-#if 0 && defined(DEBUG)
+#if 1 && defined(DEBUG)
     screen->Show( 0, std::cout );
 #endif
 

eeschema/program.h

@@ -107,13 +107,6 @@ public:
     bool    Save( FILE* aFile ) const;
 
 #if defined(DEBUG)
-    /**
-     * Function Show
-     * is used to output the object tree, currently for debugging only.
-     * @param nestLevel An aid to prettier tree indenting, and is the level
-     *          of nesting of this object within the overall tree.
-     * @param os The ostream& to output to.
-     */
     void Show( int nestLevel, std::ostream& os );
 #endif
 };
@@ -149,13 +142,6 @@ public:
     bool    Save( FILE* aFile ) const;
 
 #if defined(DEBUG)
-    /**
-     * Function Show
-     * is used to output the object tree, currently for debugging only.
-     * @param nestLevel An aid to prettier tree indenting, and is the level
-     *          of nesting of this object within the overall tree.
-     * @param os The ostream& to output to.
-     */
     void Show( int nestLevel, std::ostream& os );
 #endif
 };
@@ -287,6 +273,10 @@ public:
      */
     bool    Save( FILE* aFile ) const;
 
+#if defined(DEBUG)
+    void Show( int nestLevel, std::ostream& os );
+#endif
+
 };
 
 

eeschema/sheetlab.cpp

@@ -419,43 +419,41 @@ void WinEDA_SchematicFrame::DeleteSheetLabel( wxDC*                 DC,
  *  si DC != NULL, effacement a l'ecran du dessin
  */
 {
-    EDA_BaseStruct*       DrawStruct;
-    Hierarchical_PIN_Sheet_Struct* SheetLabel, * NextLabel;
-
     if( DC )
         RedrawOneStruct( DrawPanel, DC, SheetLabelToDel, g_XorMode );
 
-    /* Recherche de la DrawSheetStruct d'origine */
-    DrawStruct = SheetLabelToDel->m_Parent;
+    DrawSheetStruct* parent = (DrawSheetStruct*) SheetLabelToDel->m_Parent;
 
-    if( DrawStruct ) // Modification du chainage
-    {
-        if( DrawStruct->Type() != DRAW_SHEET_STRUCT_TYPE )
-        {
-            DisplayError( this,
-                         wxT( "DeleteSheetLabel error: m_Parent != DRAW_SHEET_STRUCT_TYPE" ) );
-            return;
-        }
+    wxASSERT( parent );
+    wxASSERT( parent->Type() == DRAW_SHEET_STRUCT_TYPE );
 
-        /* suppression chainage */
-        SheetLabel = ( (DrawSheetStruct*) DrawStruct )->m_Label;
-        if( SheetLabel == SheetLabelToDel )
-            ( (DrawSheetStruct*) DrawStruct )->m_Label =
-                (Hierarchical_PIN_Sheet_Struct*) SheetLabel->Pnext;
+#if 0 && defined(DEBUG)
+    std::cout << "\n\nbefore deleting:\n" << std::flush;
+    parent->Show( 0, std::cout );
+    std::cout << "\n\n\n" << std::flush;
+#endif
 
-        else
-            while( SheetLabel ) /* Examen de la liste dependante et suppression chainage */
+    Hierarchical_PIN_Sheet_Struct* label = parent->m_Label;
+
+    Hierarchical_PIN_Sheet_Struct** pprev = &parent->m_Label;
+
+    while( label )
     {
-                NextLabel = (Hierarchical_PIN_Sheet_Struct*) SheetLabel->Pnext;
-                if( NextLabel == SheetLabelToDel )
+        if( label == SheetLabelToDel )
         {
-                    SheetLabel->Pnext = NextLabel->Pnext;
-                    break;;
-                }
-                SheetLabel = NextLabel;
+            *pprev = label->Next();
+            break;
         }
 
+        pprev = (Hierarchical_PIN_Sheet_Struct**) &label->Pnext;
+        label = label->Next();
     }
 
     delete SheetLabelToDel;
+
+#if 0 && defined(DEBUG)
+    std::cout << "\n\nafter deleting:\n" << std::flush;
+    parent->Show( 0, std::cout );
+    std::cout << "~after deleting\n\n" << std::flush;
+#endif
 }