From b9f9f73c9b939cd146bfd254be436ee715525395 Mon Sep 17 00:00:00 2001
From: Tim Edwards <tim@opencircuitdesign.com>
Date: Fri, 20 Nov 2020 19:56:41 -0500
Subject: [PATCH] "Partly" corrected an issue in GDS read:  The cd_client
 record is used both for counting cells during GDS write and for saving
 geometry data from the "copyup" operator during GDS read.  The write routine
 does not clear the client record, and the read routine was checking if the
 cd_client value was default.  Corrected the resulting crash condition by
 resetting cd_client before GDS reads.  However, the underlying problem is
 that the GDS read is reading data into a cell that already exists in the
 database, and is not handling it robustly by renaming the existing cell.  So
 this should be revisited.

---
 calma/CalmaRdcl.c |  7 +++----
 calma/CalmaRdpt.c |  4 ++--
 calma/CalmaRead.c |  7 +++++++
 cif/CIFrdcl.c     | 14 +++++++-------
 commands/CmdCD.c  |  2 +-
 5 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/calma/CalmaRdcl.c b/calma/CalmaRdcl.c
index f7a80472..111f04c4 100644
--- a/calma/CalmaRdcl.c
+++ b/calma/CalmaRdcl.c
@@ -417,7 +417,7 @@ calmaParseStructure(filename)
 	/* If CDFLATGDS is already set, may need to remove	*/
 	/* existing planes and free memory.			*/
 
-	if ((cifReadCellDef->cd_client != (ClientData)CLIENTDEFAULT) &&
+	if ((cifReadCellDef->cd_client != (ClientData)0) &&
 		(cifReadCellDef->cd_flags & CDFLATGDS))
 	{
 	    Plane **cifplanes = (Plane **)cifReadCellDef->cd_client;
@@ -432,7 +432,7 @@ calmaParseStructure(filename)
                 }
             }
             freeMagic((char *)cifReadCellDef->cd_client);
-            cifReadCellDef->cd_client = (ClientData)CLIENTDEFAULT;
+            cifReadCellDef->cd_client = (ClientData)0;
 	}
 
 	TxPrintf("Saving contents of cell %s\n", cifReadCellDef->cd_name);
@@ -917,8 +917,7 @@ calmaElementSref(filename)
 
 	    for (pNum = 0; pNum < MAXCIFRLAYERS; pNum++)
 	    {
-		if ((def->cd_client != (ClientData)CLIENTDEFAULT) &&
-			(gdsplanes[pNum] != NULL))
+		if ((def->cd_client != (ClientData)0) && (gdsplanes[pNum] != NULL))
 		{
 		    gdsCopyRec.plane = cifCurReadPlanes[pNum];
 		    if (isArray)
diff --git a/calma/CalmaRdpt.c b/calma/CalmaRdpt.c
index a3f64969..4158f4f8 100644
--- a/calma/CalmaRdpt.c
+++ b/calma/CalmaRdpt.c
@@ -98,8 +98,8 @@ calmaInputRescale(n, d)
 	{
 	    /* Scale the GDS planes in this cell's cd_client record */
 	    Plane **gdsplanes = (Plane **)def->cd_client;
-	    /* Should not happen, but punt if client record is not set */
-	    if (def->cd_client != (ClientData)CLIENTDEFAULT)
+	    /* Should not happen, but punt if client record is not set; */
+	    if (def->cd_client != (ClientData)0)
 		CIFScalePlanes(n, d, gdsplanes);
 	}
     }
diff --git a/calma/CalmaRead.c b/calma/CalmaRead.c
index a145078b..822115a2 100644
--- a/calma/CalmaRead.c
+++ b/calma/CalmaRead.c
@@ -78,6 +78,7 @@ bool CalmaPostOrder = FALSE;		/* If TRUE, forces the GDS parser to
 					 * Added by Nishit 8/16/2004
 					 */
 extern void calmaUnexpected();
+extern int calmaWriteInitFunc();
 
 bool calmaParseUnits();
 
@@ -172,6 +173,12 @@ CalmaReadFile(file, filename)
     CalmaPolygonCount = 0;
     CalmaPathCount = 0;
 
+    /* Reset cd_client pointers (using init function from CalmaWrite.c) */
+    /* This is in case a cell already in memory is being referenced;	*/
+    /* it is probably better to avoid those kinds of naming collisions	*/
+    /* though. . . 							*/
+    (void) DBCellSrDefs(0, calmaWriteInitFunc, (ClientData) NULL);
+
     HashInit(&calmaDefInitHash, 32, 0);
     calmaLApresent = FALSE;
     calmaInputFile = file;
diff --git a/cif/CIFrdcl.c b/cif/CIFrdcl.c
index 10dd823a..02b07f65 100644
--- a/cif/CIFrdcl.c
+++ b/cif/CIFrdcl.c
@@ -603,13 +603,13 @@ CIFPaintCurrent(filetype)
 		Plane **parray;
 		extern char *(cifReadLayers[MAXCIFRLAYERS]);
 
-		/* NOTE:  There should be no need to check for cd_client
-		 * here as cd_client should not be CLIENTDEFAULT if CDFLATGDS
-		 * is set in flags.  This condition has occurred, though, and
-		 * needs to be debugged.
+		/* NOTE:  The condition cd_client == 0 when CDFLATGDS
+		 * indicates that the cell was already in memory when the
+		 * GDS was read.  This condition should be properly caught
+		 * and handled.
 		 */
 		if ((cifReadCellDef->cd_flags & CDFLATGDS) &&
-				(cifReadCellDef->cd_client != (ClientData)CLIENTDEFAULT))
+				(cifReadCellDef->cd_client != (ClientData)0))
 		    parray = (Plane **)cifReadCellDef->cd_client;
 		else
 		{
@@ -1499,7 +1499,7 @@ CIFReadCellCleanup(filetype)
 	    UndoDisable();
 
 	    /* cifplanes should be valid, but don't crash magic if not */
-	    if (cifplanes != (Plane **)CLIENTDEFAULT)
+	    if (cifplanes != (Plane **)0)
 	    {
 
 		for (pNum = 0; pNum < MAXCIFRLAYERS; pNum++)
@@ -1512,7 +1512,7 @@ CIFReadCellCleanup(filetype)
 		}
 		freeMagic((char *)def->cd_client);
 	    }
-	    def->cd_client = (ClientData)CLIENTDEFAULT;
+	    def->cd_client = (ClientData)0;
 
 #if 0
 	    /* If the CDFLATTENED flag was not set, then this geometry	*/
diff --git a/commands/CmdCD.c b/commands/CmdCD.c
index fda146a9..c7710dde 100644
--- a/commands/CmdCD.c
+++ b/commands/CmdCD.c
@@ -3454,7 +3454,7 @@ CmdDrc(w, cmd)
 	    }
 
 #ifdef MAGIC_WRAPPER
-	    if (count_total >= 0)
+	    if ((count_total >= 0) || (!dolist))
 	    {
 		if (dolist)
 		    Tcl_SetObjResult(magicinterp, Tcl_NewIntObj(count_total));