From: Scott Wood <scott@thor.buserror.net>
Date: Mon, 7 Apr 2008 04:17:10 +0000 (-0500)
Subject: idlcomp: valgrind fixes
X-Git-Url: http://git.buserror.net/cgi-bin/gitweb.cgi?p=polintos%2Fscott%2Fpriv.git;a=commitdiff_plain;h=eec2efc14a0d624d475dc89e2bcb962e7c081df5

idlcomp: valgrind fixes

The NameSpace::dying flag is broken; if there's another reference to
a child, it will persist with a bad ns pointer.  Instead, have the
namespace destructor loop through its children and call del().

Remove an unnecessary retention in Symbol constructor.
---

diff --git a/idlcomp/idlc.h b/idlcomp/idlc.h
index e030bbe..a4ca39a 100644
--- a/idlcomp/idlc.h
+++ b/idlcomp/idlc.h
@@ -447,6 +447,8 @@ typedef Ref<IDList> IDListRef;
 class NameSpace;
 class LangCallback;
 
+typedef Ref<NameSpace> NameSpaceRef;
+
 // This is incremented when a chain of symbols is traversed to reset
 // detection of already-visited symbols.  It is assumed that this
 // will not have to happen billions of times.
@@ -488,9 +490,6 @@ public:
 	
 	Symbol(const String *_name) : name(_name)
 	{
-		if (_name)
-			name->retain();
-
 		init();
 	}
 
@@ -585,8 +584,6 @@ struct InvalidArgument {
 struct UserError {
 };
 
-typedef Ref<NameSpace> NameSpaceRef;
-
 class NameSpace : public virtual Symbol {
 protected:
 	// Filesystem path to the external symbol storage, or NULL
@@ -617,23 +614,15 @@ protected:
 	list<NameSpaceRef> imports;
 
 public:
-	// This is set in the destructor, so the contents of the namespace
-	// don't try to remove themselves from the namespace when destructed
-	// due to the destruction of map.
-	int dying;
-	
 	// This is a counter for generating unique names for anonymous
 	// members of the namespace.
 	int anon;
 	
- 	NameSpace() : dying(0), anon(0)
+ 	NameSpace() : anon(0)
 	{
 	}
 	
-	virtual ~NameSpace()
-	{
-		dying = 1;
-	}
+	virtual ~NameSpace();
 	
 	// Return a description of the type of namespace, for
 	// error messages.
@@ -704,13 +693,12 @@ public:
 	// SymbolNotFound is thrown if sym is not in this namespace.
 	virtual void del(Symbol *sym)
 	{
-		fprintf(stderr, "Removing symbol %s\n",
-		        sym->get_fq_name()->flatten()->c_str());
-	
-		if (tbl.erase(sym->name) == 0)
+		if (sym->ns != this)
 			throw SymbolNotFound();
-		
+
 		sym->ns = NULL;
+		int ret = tbl.erase(sym->name);
+		assert(ret != 0);
 	}
 
 private:	
@@ -1280,7 +1268,7 @@ public:
 
 class Struct;
 typedef Ref<Struct> StructRef;
-extern Struct *System_VStruct;
+extern StructRef System_VStruct;
 
 // FIXME: typedefed superstructs
 class Struct : public NameSpace, public Type, public Def {
@@ -1651,7 +1639,7 @@ typedef Ref<Method> MethodRef;
 class Interface;
 typedef Ref<Interface> InterfaceRef;
 
-extern Interface *System_Object;
+extern InterfaceRef System_Object;
 
 // FIXME: typedefed superinterfaces
 class Interface : public NameSpace, public Type, public Def {
diff --git a/idlcomp/main.cc b/idlcomp/main.cc
index c21532e..a02453c 100644
--- a/idlcomp/main.cc
+++ b/idlcomp/main.cc
@@ -129,8 +129,8 @@ const char *cmdname, *output_dir, *cur_input_file = "<none>", *output_ns_name;
 list<const char *> inputs;
 list<NameSpaceRef> nspace_stack;
 Language *first_lang, *output_lang;
-Interface *System_Object;
-Struct *System_VStruct;
+InterfaceRef System_Object;
+StructRef System_VStruct;
 AutoReleasePool autorelease_pool;
 
 void print_usage()
@@ -787,6 +787,7 @@ int main(int argc, const char **argv)
 {
 	int ret = run_idlc(argc, argv);
 	autorelease_pool.clean();
+
 	return ret;
 }
 
diff --git a/idlcomp/namespace.cc b/idlcomp/namespace.cc
index b681d91..a2fe015 100644
--- a/idlcomp/namespace.cc
+++ b/idlcomp/namespace.cc
@@ -276,7 +276,7 @@ Symbol::~Symbol()
 	if (lang_priv)
 		lang_priv->release();
 
-	if (ns && !ns->dying) {
+	if (ns) {
 		try {
 			ns->del(this);
 		}
@@ -287,6 +287,29 @@ Symbol::~Symbol()
 	}
 }
 
+NameSpace::~NameSpace()
+{
+	tbl_type::iterator i = tbl.begin();
+	while (i != tbl.end()) {
+		Symbol *sym = (*i).second;
+		++i;
+		
+		try {
+			del(sym);
+		}
+
+		catch (SymbolNotFound) {
+			fprintf(stderr, "SymbolNotFound in NameSpace::~NameSpace(), cannot propagate\n");
+		}
+
+		catch (InternalError &e) {
+			fprintf(stderr, "InternalError %s:%d in NameSpace::~NameSpace(), cannot propagate\n", e.file, e.line);
+		}
+	}
+	
+	assert(tbl.empty());
+}
+
 StrList *Symbol::get_fq_name(const char *append, bool not_last) const
 {
 	StrList *ret;