From bb7362cebabc96bf15adaa562299ee99ce8c6fa9 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Thu, 8 Sep 2022 23:17:41 +0100
Subject: [PATCH] Don't allow a SaveAs to overwrite the root sheet.

Also prevents EESchema from loading sheets which recurse.

Fixes https://gitlab.com/kicad/code/kicad/issues/10872

(cherry picked from commit d030f03844ab741957b0f66d24cdcdf3f9c25936)
---
 eeschema/eeschema.cpp                         | 14 +++++++
 .../sch_plugins/kicad/sch_sexpr_plugin.cpp    | 38 ++++++++++++++++---
 eeschema/sch_plugins/kicad/sch_sexpr_plugin.h |  2 +-
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/eeschema/eeschema.cpp b/eeschema/eeschema.cpp
index 3b0de10366..bf283e4aab 100644
--- a/eeschema/eeschema.cpp
+++ b/eeschema/eeschema.cpp
@@ -321,7 +321,21 @@ void IFACE::SaveFileAs( const wxString& aProjectBasePath, const wxString& aProje
         ext == KiCadSchematicFileExtension + BackupFileSuffix )
     {
         if( destFile.GetName() == aProjectName )
+        {
             destFile.SetName( aNewProjectName  );
+        }
+        else if( destFile.GetName() == aNewProjectName )
+        {
+            wxString msg;
+
+            if( !aErrors.empty() )
+                aErrors += "\n";
+
+            msg.Printf( _( "Cannot copy file '%s' as it will be overwritten by the new root "
+                           "sheet file." ), destFile.GetFullPath() );
+            aErrors += msg;
+            return;
+        }
 
         // Sheet paths when auto-generated are relative to the root, so those will stay
         // pointing to whatever they were pointing at.
diff --git a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp
index 34d85c3f4c..24e3a819bf 100644
--- a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp
+++ b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp
@@ -572,7 +572,7 @@ SCH_SHEET* SCH_SEXPR_PLUGIN::Load( const wxString& aFileName, SCHEMATIC* aSchema
 
         newSheet->SetFileName( relPath.GetFullPath() );
         m_rootSheet = newSheet.get();
-        loadHierarchy( newSheet.get() );
+        loadHierarchy( SCH_SHEET_PATH(), newSheet.get() );
 
         // If we got here, the schematic loaded successfully.
         sheet = newSheet.release();
@@ -583,7 +583,7 @@ SCH_SHEET* SCH_SEXPR_PLUGIN::Load( const wxString& aFileName, SCHEMATIC* aSchema
         wxCHECK_MSG( aSchematic->IsValid(), nullptr, "Can't append to a schematic with no root!" );
         m_rootSheet = &aSchematic->Root();
         sheet = aAppendToMe;
-        loadHierarchy( sheet );
+        loadHierarchy( SCH_SHEET_PATH(), sheet );
     }
 
     wxASSERT( m_currentPath.size() == 1 );  // only the project path should remain
@@ -596,7 +596,7 @@ SCH_SHEET* SCH_SEXPR_PLUGIN::Load( const wxString& aFileName, SCHEMATIC* aSchema
 
 // Everything below this comment is recursive.  Modify with care.
 
-void SCH_SEXPR_PLUGIN::loadHierarchy( SCH_SHEET* aSheet )
+void SCH_SEXPR_PLUGIN::loadHierarchy( const SCH_SHEET_PATH& aParentSheetPath, SCH_SHEET* aSheet )
 {
     SCH_SCREEN* screen = nullptr;
 
@@ -618,7 +618,30 @@ void SCH_SEXPR_PLUGIN::loadHierarchy( SCH_SHEET* aSheet )
         wxLogTrace( traceSchLegacyPlugin, "Current path   '%s'", m_currentPath.top() );
         wxLogTrace( traceSchLegacyPlugin, "Loading        '%s'", fileName.GetFullPath() );
 
-        m_rootSheet->SearchHierarchy( fileName.GetFullPath(), &screen );
+        SCH_SHEET_PATH ancestorSheetPath = aParentSheetPath;
+
+        while( !ancestorSheetPath.empty() )
+        {
+            if( ancestorSheetPath.LastScreen()->GetFileName() == fileName.GetFullPath() )
+            {
+                if( !m_error.IsEmpty() )
+                    m_error += "\n";
+
+                m_error += wxString::Format( _( "Could not load sheet '%s' because it already "
+                                                "appears as a direct ancestor in the schematic "
+                                                "hierarchy." ),
+                                             fileName.GetFullPath() );
+
+                fileName = wxEmptyString;
+
+                break;
+            }
+
+            ancestorSheetPath.pop_back();
+        }
+
+        if( ancestorSheetPath.empty() )
+            m_rootSheet->SearchHierarchy( fileName.GetFullPath(), &screen );
 
         if( screen )
         {
@@ -659,7 +682,10 @@ void SCH_SEXPR_PLUGIN::loadHierarchy( SCH_SHEET* aSheet )
                 aSheet->GetScreen()->SetFileExists( false );
             }
 
-            // This was moved out of the try{} block so that any sheets definitionsthat
+            SCH_SHEET_PATH currentSheetPath = aParentSheetPath;
+            currentSheetPath.push_back( aSheet );
+
+            // This was moved out of the try{} block so that any sheet definitions that
             // the plugin fully parsed before the exception was raised will be loaded.
             for( SCH_ITEM* aItem : aSheet->GetScreen()->Items().OfType( SCH_SHEET_T ) )
             {
@@ -667,7 +693,7 @@ void SCH_SEXPR_PLUGIN::loadHierarchy( SCH_SHEET* aSheet )
                 SCH_SHEET* sheet = static_cast<SCH_SHEET*>( aItem );
 
                 // Recursion starts here.
-                loadHierarchy( sheet );
+                loadHierarchy( currentSheetPath, sheet );
             }
         }
 
diff --git a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.h b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.h
index 3506d86724..775a84d743 100644
--- a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.h
+++ b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.h
@@ -137,7 +137,7 @@ public:
     static void FormatLibSymbol( LIB_SYMBOL* aPart, OUTPUTFORMATTER& aFormatter );
 
 private:
-    void loadHierarchy( SCH_SHEET* aSheet );
+    void loadHierarchy( const SCH_SHEET_PATH& aParentSheetPath, SCH_SHEET* aSheet );
     void loadFile( const wxString& aFileName, SCH_SHEET* aSheet );
 
     void saveSymbol( SCH_SYMBOL* aSymbol, SCH_SHEET_PATH* aSheetPath, int aNestLevel );