Merge pull request #24 from torpedro/mem-leaks

Fix memory leaks

.travis.yml

@@ -4,17 +4,30 @@ language: cpp
 install:
   - sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test
   - sudo apt-get -qq update
-  - sudo apt-get install -y bison flex
   - sudo apt-get install -y g++-4.8 libstdc++-4.8-dev
+  - sudo apt-get install -y flex valgrind
   - sudo update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-4.8 90
   - sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-4.8 90
+  
+  # Install bison 3.0.4.
+  - wget http://ftp.gnu.org/gnu/bison/bison-3.0.4.tar.gz
+  - tar -xvzf bison-3.0.4.tar.gz
+  - cd bison-3.0.4
+  - ./configure && make && sudo make install
+  - cd ..
+
+  # Show installed versions.
   - which g++
   - g++ -v
+  - bison --version
+  - flex --version
+  - valgrind --version
 
 compiler:
   - gcc
   - clang
 
 script:
+  - make cleanall
   - make
   - make test

src/SQLParser.cpp

@@ -28,11 +28,12 @@ namespace hsql {
     // Parser and return early if it failed.
     if (hsql_parse(&result, scanner)) {
       // Returns an error stmt object.
+      hsql__delete_buffer(state, scanner);
+      hsql_lex_destroy(scanner);
       return result;
     }
 
     hsql__delete_buffer(state, scanner);
-
     hsql_lex_destroy(scanner);
     return result;
   }

src/SQLParserResult.cpp

@@ -18,7 +18,7 @@ namespace hsql {
       delete statement;
     }
 
-    delete errorMsg_;
+    free(errorMsg_);
   }
 
   void SQLParserResult::addStatement(SQLStatement* stmt) {
@@ -57,7 +57,7 @@ namespace hsql {
     isValid_ = isValid;
   }
 
-  void SQLParserResult::setErrorDetails(const char* errorMsg, int errorLine, int errorColumn) {
+  void SQLParserResult::setErrorDetails(char* errorMsg, int errorLine, int errorColumn) {
     errorMsg_ = errorMsg;
     errorLine_ = errorLine;
     errorColumn_ = errorColumn;

src/SQLParserResult.h

@@ -47,7 +47,8 @@ namespace hsql {
     void setIsValid(bool isValid);
 
     // Set the details of the error, if available.
-    void setErrorDetails(const char* errorMsg, int errorLine, int errorColumn);
+    // Takes ownership of errorMsg.
+    void setErrorDetails(char* errorMsg, int errorLine, int errorColumn);
 
 
    private:
@@ -58,7 +59,7 @@ namespace hsql {
     bool isValid_;
 
     // Error message, if an error occurred.
-    const char* errorMsg_;
+    char* errorMsg_;
 
     // Line number of the occurrance of the error in the query.
     int errorLine_;

src/parser/Makefile

@@ -2,9 +2,11 @@
 all: bison_parser.cpp flex_lexer.cpp
 
 bison_parser.cpp: bison_parser.y
+	@bison --version | head -n 1
 	bison bison_parser.y -v
 
 flex_lexer.cpp: flex_lexer.l
+	@flex --version
 	flex flex_lexer.l
 
 clean:

src/parser/bison_parser.h

@@ -48,7 +48,7 @@
 extern int hsql_debug;
 #endif
 /* "%code requires" blocks.  */
-#line 40 "bison_parser.y" /* yacc.c:1909  */
+#line 41 "bison_parser.y" /* yacc.c:1909  */
 
 // %code requires block	
 
@@ -209,7 +209,7 @@ extern int hsql_debug;
 typedef union HSQL_STYPE HSQL_STYPE;
 union HSQL_STYPE
 {
-#line 99 "bison_parser.y" /* yacc.c:1909  */
+#line 100 "bison_parser.y" /* yacc.c:1909  */
 
 	double fval;
 	int64_t ival;

src/parser/bison_parser.y

@@ -19,6 +19,7 @@
 using namespace hsql;
 
 int yyerror(YYLTYPE* llocp, SQLParserResult** result, yyscan_t scanner, const char *msg) {
+	delete *result;
 
 	SQLParserResult* list = new SQLParserResult();
 	list->setIsValid(false);
@@ -133,6 +134,19 @@ int yyerror(YYLTYPE* llocp, SQLParserResult** result, yyscan_t scanner, const ch
 }
 
 
+/*********************************
+ ** Descrutor symbols
+ *********************************/
+%destructor { } <fval> <ival> <uval> <bval> <order_type>
+%destructor { free( ($$) ); } <sval>
+%destructor {
+	for (auto ptr : *($$)) {
+		delete ptr;
+	}
+	delete ($$);
+} <str_vec> <table_vec> <column_vec> <update_vec> <expr_vec>
+%destructor { delete ($$); } <*>
+
 
 /*********************************
  ** Token Definition
@@ -303,7 +317,7 @@ import_file_type:
 	;
 
 file_path:
-		string_literal { $$ = $1->name; }
+		string_literal { $$ = strdup($1->name); delete $1; }
 	;
 
 

src/sql/CreateStatement.h

@@ -34,8 +34,8 @@ namespace hsql {
 
     CreateType type;
     bool ifNotExists; // default: false
-    const char* filePath; // default: NULL
-    const char* tableName; // default: NULL
+    char* filePath; // default: NULL
+    char* tableName; // default: NULL
     std::vector<ColumnDefinition*>* columns; // default: NULL
   };
 

src/sql/DropStatement.h

@@ -20,7 +20,7 @@ namespace hsql {
     virtual ~DropStatement();
 
     EntityType type;
-    const char* name;
+    char* name;
   };
 
 } // namespace hsql

src/sql/ExecuteStatement.h

@@ -12,7 +12,7 @@ namespace hsql {
     ExecuteStatement();
     virtual ~ExecuteStatement();
 
-    const char* name;
+    char* name;
     std::vector<Expr*>* parameters;
   };
 

src/sql/Expr.cpp

@@ -16,8 +16,9 @@ namespace hsql {
   Expr::~Expr() {
     delete expr;
     delete expr2;
-    delete name;
-    delete table;
+    free(name);
+    free(table);
+    free(alias);
   }
 
   Expr* Expr::makeOpUnary(OperatorType op, Expr* expr) {
@@ -123,7 +124,7 @@ namespace hsql {
 
   char* substr(const char* source, int from, int to) {
     int len = to - from;
-    char* copy = new char[len + 1];
+    char* copy = (char*) malloc(len + 1);;
     strncpy(copy, source + from, len);
     copy[len] = '\0';
     return copy;

src/sql/InsertStatement.h

@@ -19,7 +19,7 @@ namespace hsql {
     virtual ~InsertStatement();
 
     InsertType type;
-    const char* tableName;
+    char* tableName;
     std::vector<char*>* columns;
     std::vector<Expr*>* values;
     SelectStatement* select;

src/sql/PrepareStatement.h

@@ -23,8 +23,12 @@ namespace hsql {
      */
     void setPlaceholders(std::vector<void*> ph);
 
-    const char* name;
+    char* name;
     SQLParserResult* query;
+
+    // The expressions are not owned by this statement.
+    // Rather they are owned by the query and destroyed, when
+    // the query is destroyed.
     std::vector<Expr*> placeholders;
   };
 

src/sql/statements.cpp

@@ -19,7 +19,7 @@ namespace hsql {
     type(type) {};
 
   ColumnDefinition::~ColumnDefinition() {
-    delete name;
+    free(name);
   }
 
   // CreateStatemnet
@@ -32,9 +32,15 @@ namespace hsql {
     columns(NULL) {};
 
   CreateStatement::~CreateStatement() {
+    free(filePath);
+    free(tableName);
+
+    if (columns != NULL) {
+      for (ColumnDefinition* def : *columns) {
+        delete def;
+      }
       delete columns;
-    delete filePath;
-    delete tableName;
+    }
   }
 
   // DeleteStatement
@@ -44,7 +50,7 @@ namespace hsql {
     expr(NULL) {};
 
   DeleteStatement::~DeleteStatement() {
-    delete tableName;
+    free(tableName);
     delete expr;
   }
 
@@ -55,7 +61,7 @@ namespace hsql {
     name(NULL) {}
 
   DropStatement::~DropStatement() {
-    delete name;
+    free(name);
   }
 
   // ExecuteStatement
@@ -65,9 +71,15 @@ namespace hsql {
     parameters(NULL) {}
 
   ExecuteStatement::~ExecuteStatement() {
-    delete name;
+    free(name);
+
+    if (parameters != NULL) {
+      for (Expr* param : *parameters) {
+        delete param;
+      }
       delete parameters;
     }
+  }
 
   // ImportStatement
   ImportStatement::ImportStatement(ImportType type) :
@@ -91,10 +103,22 @@ namespace hsql {
     select(NULL) {}
 
   InsertStatement::~InsertStatement() {
-    delete tableName;
-    delete columns;
-    delete values;
+    free(tableName);
     delete select;
+
+    if (columns != NULL) {
+      for (char* column : *columns) {
+        free(column);
+      }
+      delete columns;
+    }
+
+    if (values != NULL) {
+      for (Expr* expr : *values) {
+        delete expr;
+      }
+      delete values;
+    }
   }
 
   // PrepareStatement
@@ -105,7 +129,7 @@ namespace hsql {
 
   PrepareStatement::~PrepareStatement() {
     delete query;
-    delete name;
+    free(name);
   }
 
   void PrepareStatement::setPlaceholders(std::vector<void*> ph) {
@@ -142,8 +166,14 @@ namespace hsql {
     having(NULL) {}
 
   GroupByDescription::~GroupByDescription() {
-    delete columns;
     delete having;
+
+    if (columns != NULL) {
+      for (Expr* expr : *columns) {
+        delete expr;
+      }
+      delete columns;
+    }
   }
 
   // SelectStatement
@@ -160,11 +190,19 @@ namespace hsql {
 
   SelectStatement::~SelectStatement() {
     delete fromTable;
-    delete selectList;
     delete whereClause;
     delete groupBy;
+    delete unionSelect;
     delete order;
     delete limit;
+
+    // Delete each element in the select list.
+    if (selectList != NULL) {
+      for (Expr* expr : *selectList) {
+        delete expr;
+      }
+      delete selectList;
+    }
   }
 
   // UpdateStatement
@@ -176,8 +214,16 @@ namespace hsql {
 
   UpdateStatement::~UpdateStatement() {
     delete table;
-    delete updates;
     delete where;
+
+    if (updates != NULL) {
+      for (UpdateClause* update : *updates) {
+        free(update->column);
+        delete update->value;
+        delete update;
+      }
+      delete updates;
+    }
   }
 
   // TableRef
@@ -191,11 +237,20 @@ namespace hsql {
     join(NULL) {}
 
   TableRef::~TableRef() {
-    delete name;
-    delete alias;
+    free(schema);
+    free(name);
+    free(alias);
+
     delete select;
+    delete join;
+
+    if (list != NULL) {
+      for (TableRef* table : *list) {
+        delete table;
+      }
       delete list;
     }
+  }
 
   bool TableRef::hasSchema() {
     return schema != NULL;

test/lib/helper.h

@@ -2,21 +2,21 @@
 #define __HELPER_H__
 
 
-#define TEST_PARSE_SQL_QUERY(query, outputVar, numStatements) \
-	const SQLParserResult* outputVar = SQLParser::parseSQLString(query); \
-	ASSERT(outputVar->isValid()); \
-	ASSERT_EQ(outputVar->size(), numStatements);
+#define TEST_PARSE_SQL_QUERY(query, result, numStatements) \
+	const SQLParserResult* result = SQLParser::parseSQLString(query); \
+	ASSERT(result->isValid()); \
+	ASSERT_EQ(result->size(), numStatements);
 
 
-#define TEST_PARSE_SINGLE_SQL(query, stmtType, stmtClass, outputVar) \
-	TEST_PARSE_SQL_QUERY(query, stmt_list, 1); \
-    ASSERT_EQ(stmt_list->getStatement(0)->type(), stmtType); \
-    const stmtClass* outputVar = (const stmtClass*) stmt_list->getStatement(0);
+#define TEST_PARSE_SINGLE_SQL(query, stmtType, stmtClass, result, outputVar) \
+	TEST_PARSE_SQL_QUERY(query, result, 1); \
+    ASSERT_EQ(result->getStatement(0)->type(), stmtType); \
+    const stmtClass* outputVar = (const stmtClass*) result->getStatement(0);
 
 
-#define TEST_CAST_STMT(stmt_list, stmt_index, stmtType, stmtClass, outputVar) \
-    ASSERT_EQ(stmt_list->getStatement(stmt_index)->type(), stmtType); \
-    const stmtClass* outputVar = (const stmtClass*) stmt_list->getStatement(stmt_index);
+#define TEST_CAST_STMT(result, stmt_index, stmtType, stmtClass, outputVar) \
+    ASSERT_EQ(result->getStatement(stmt_index)->type(), stmtType); \
+    const stmtClass* outputVar = (const stmtClass*) result->getStatement(stmt_index);
 
 
 #endif

test/lib/select.cpp

@@ -7,15 +7,28 @@
 using namespace hsql;
 
 TEST(SelectTest) {
-  TEST_PARSE_SINGLE_SQL("SELECT * FROM students;", kStmtSelect, SelectStatement, stmt);
+  TEST_PARSE_SINGLE_SQL(
+    "SELECT * FROM students;",
+    kStmtSelect,
+    SelectStatement,
+    result,
+    stmt);
 
   ASSERT_NULL(stmt->whereClause);
   ASSERT_NULL(stmt->groupBy);
+
+  delete result;
 }
 
 
 TEST(SelectHavingTest) {
-  TEST_PARSE_SINGLE_SQL("SELECT city, AVG(grade) AS avg_grade FROM students GROUP BY city HAVING AVG(grade) < 2.0", kStmtSelect, SelectStatement, stmt);
+  TEST_PARSE_SINGLE_SQL(
+    "SELECT city, AVG(grade) AS avg_grade FROM students GROUP BY city HAVING AVG(grade) < 2.0",
+    kStmtSelect,
+    SelectStatement,
+    result,
+    stmt);
+
   ASSERT_FALSE(stmt->selectDistinct);
 
   GroupByDescription* group = stmt->groupBy;
@@ -24,23 +37,39 @@ TEST(SelectHavingTest) {
   ASSERT(group->having->isSimpleOp('<'));
   ASSERT(group->having->expr->isType(kExprFunctionRef));
   ASSERT(group->having->expr2->isType(kExprLiteralFloat));
+
+  delete result;
 }
 
 
 TEST(SelectDistinctTest) {
-  TEST_PARSE_SINGLE_SQL("SELECT DISTINCT grade, city FROM students;", kStmtSelect, SelectStatement, stmt);
+  TEST_PARSE_SINGLE_SQL(
+    "SELECT DISTINCT grade, city FROM students;",
+    kStmtSelect,
+    SelectStatement, 
+    result,
+    stmt);
 
   ASSERT(stmt->selectDistinct);
   ASSERT_NULL(stmt->whereClause);
+
+  delete result;
 }
 
 TEST(SelectGroupDistinctTest) {
-  TEST_PARSE_SINGLE_SQL("SELECT city, COUNT(name), COUNT(DISTINCT grade) FROM students GROUP BY city;", kStmtSelect, SelectStatement, stmt);
+  TEST_PARSE_SINGLE_SQL(
+    "SELECT city, COUNT(name), COUNT(DISTINCT grade) FROM students GROUP BY city;",
+    kStmtSelect,
+    SelectStatement,
+    result,
+    stmt);
 
   ASSERT_FALSE(stmt->selectDistinct);
   ASSERT_EQ(stmt->selectList->size(), 3);
   ASSERT(!stmt->selectList->at(1)->distinct);
   ASSERT(stmt->selectList->at(2)->distinct);
+
+  delete result;
 }
 
 

test/lib/test.cpp

@@ -8,13 +8,13 @@ class TestsManager {
   // http://www.parashift.com/c++-faq-lite/static-init-order-on-first-use.html
  public:
   static std::vector<std::string>& testNames() {
-    static std::vector<std::string>* _testNames = new std::vector<std::string>;
-    return *_testNames;
+    static std::vector<std::string> testNames;
+    return testNames;
   }
 
   static std::vector<void (*)(void)>& tests() {
-    static std::vector<void (*)(void)>* tests = new std::vector<void (*)(void)>;
-    return *tests;
+    static std::vector<void (*)(void)> tests;
+    return tests;
   }
 };
 

test/lib/test.h

@@ -8,7 +8,9 @@
 
 #define TEST(name) \
 	void name();\
-	namespace g_dummy { int _##name = AddTest(name, #name); } \
+	namespace g_dummy {\
+		int _##name = AddTest(name, #name);\
+	}\
 	void name()
 
 

test/lib/valid_queries.sql

@@ -6,7 +6,7 @@ SELECT * from "table" JOIN table2 ON a = b WHERE (b OR NOT a) AND a = 12.5
 (SELECT a FROM foo WHERE a > 12 OR b > 3 AND c NOT LIKE 's%' LIMIT 10);
 SELECT * FROM "table" LIMIT 10 OFFSET 10; SELECT * FROM second;
 SELECT * FROM t1 UNION SELECT * FROM t2 ORDER BY col1;
-# SELECT * FROM t1 UNION (SELECT * FROM t2 UNION SELECT * FROM t3) ORDER BY col1; 
+-- SELECT * FROM t1 UNION (SELECT * FROM t2 UNION SELECT * FROM t3) ORDER BY col1; 
 # JOIN
 SELECT t1.a, t1.b, t2.c FROM "table" AS t1 JOIN (SELECT * FROM foo JOIN bar ON foo.id = bar.id) t2 ON t1.a = t2.b WHERE (t1.b OR NOT t1.a) AND t2.c = 12.5
 SELECT * FROM t1 JOIN t2 ON c1 = c2;
@@ -37,3 +37,10 @@ PREPARE prep2 { INSERT INTO test VALUES (?, 0, 0); INSERT INTO test VALUES (0, ?
 EXECUTE prep_inst(1, 2, 3);
 EXECUTE prep;
 DEALLOCATE PREPARE prep;
+# Error expeced
+!
+!1
+!gibberish;
+!SELECT abc;
+!CREATE TABLE "table" FROM TBL FILE 'students.tbl';SELECT 1
+!CREATE TABLE "table" FROM TBL FILE 'students.tbl';1

test/sql_grammar_test.cpp

@@ -18,9 +18,12 @@ std::vector<std::string> readlines(std::string path) {
         std::istringstream iss(line);
 
         // Skip comments
-        if (line[0] != '#') {
-            lines.push_back(line);
+        if (line[0] == '#' || 
+            (line[0] == '-' && line[1] == '-')) {
+            continue;
         }
+        
+        lines.push_back(line);
     }
     return lines;
 }
@@ -33,14 +36,14 @@ int main(int argc, char *argv[]) {
         return -1;
     }
 
-    bool expectFalse = false;
+    bool globalExpectFalse = false;
     bool useFile = false;
     std::string filePath = "";
     
     // Parse command line arguments
     int i = 1;
     for (; i < argc; ++i) {
-        if (STREQ(argv[i], "--false")) expectFalse = true;
+        if (STREQ(argv[i], "--false")) globalExpectFalse = true;
         else if (STREQ(argv[i], "-f")) {
             useFile = true;
             filePath = argv[++i];
@@ -62,6 +65,12 @@ int main(int argc, char *argv[]) {
     // Execute queries
     int numFailed = 0;
     for (std::string sql : queries) {
+        bool expectFalse = globalExpectFalse;
+        if (sql.at(0) == '!') {
+            expectFalse = !expectFalse;
+            sql = sql.substr(1);
+        }
+
         // Measuring the parsing time
         std::chrono::time_point<std::chrono::system_clock> start, end;
         start = std::chrono::system_clock::now();
@@ -82,6 +91,8 @@ int main(int argc, char *argv[]) {
             // TODO: indicate whether expectFalse was set
             printf("\033[0;32m{      ok} (%.1fus)\033[0m %s\n", us, sql.c_str());
         }
+
+        delete result;
     }
 
     if (numFailed == 0) {

test/sql_tests.cpp

@@ -22,6 +22,8 @@ TEST(DeleteStatementTest) {
 	ASSERT(stmt->expr->isType(kExprOperator));
 	ASSERT_STREQ(stmt->expr->expr->name, "grade");
 	ASSERT_EQ(stmt->expr->expr2->fval, 2.0);
+
+	delete result;
 }
 
 TEST(CreateStatementTest) {
@@ -43,6 +45,8 @@ TEST(CreateStatementTest) {
 	ASSERT_EQ(stmt->columns->at(1)->type, ColumnDefinition::INT);
 	ASSERT_EQ(stmt->columns->at(2)->type, ColumnDefinition::INT);
 	ASSERT_EQ(stmt->columns->at(3)->type, ColumnDefinition::DOUBLE);
+
+	delete result;
 }
 
 
@@ -69,25 +73,40 @@ TEST(UpdateStatementTest) {
 	ASSERT(stmt->where->isType(kExprOperator));
 	ASSERT(stmt->where->isSimpleOp('='));
 	ASSERT_STREQ(stmt->where->expr->name, "name");
-	ASSERT_STREQ(stmt->where->expr2->name, "Max Mustermann");
+	ASSERT_STREQ(stmt->where->expr2->name, "Max Mustermann");\
 
+	delete result;
 }
 
 
 TEST(InsertStatementTest) {
-	TEST_PARSE_SINGLE_SQL("INSERT INTO students VALUES ('Max Mustermann', 12345, 'Musterhausen', 2.0)", kStmtInsert, InsertStatement, stmt);
+	TEST_PARSE_SINGLE_SQL(
+		"INSERT INTO students VALUES ('Max Mustermann', 12345, 'Musterhausen', 2.0)",
+		kStmtInsert,
+		InsertStatement,
+		result,
+		stmt);
 
 	ASSERT_EQ(stmt->values->size(), 4);
 	// TODO
+
+	delete result;
 }
 
 
 TEST(DropTableStatementTest) {
-	TEST_PARSE_SINGLE_SQL("DROP TABLE students", kStmtDrop, DropStatement, stmt);
+	TEST_PARSE_SINGLE_SQL(
+		"DROP TABLE students",
+		kStmtDrop,
+		DropStatement,
+		result,
+		stmt);
 
 	ASSERT_EQ(stmt->type, DropStatement::kTable);
 	ASSERT_NOTNULL(stmt->name);
 	ASSERT_STREQ(stmt->name, "students");
+
+	delete result;
 }
 
 
@@ -134,12 +153,16 @@ TEST(PrepareStatementTest) {
 	// Deallocate Statement
 	ASSERT_EQ(drop->type, DropStatement::kPreparedStatement);
 	ASSERT_STREQ(drop->name, "stmt");
+
+	delete result;
 }
 
 
 TEST(ExecuteStatementTest) {
-	TEST_PARSE_SINGLE_SQL("EXECUTE test(1, 2);", kStmtExecute, ExecuteStatement, stmt);
+	TEST_PARSE_SINGLE_SQL("EXECUTE test(1, 2);", kStmtExecute, ExecuteStatement, result, stmt);
 
 	ASSERT_STREQ(stmt->name, "test");
 	ASSERT_EQ(stmt->parameters->size(), 2);
+
+	delete result;
 }

test/test.sh

@@ -1,15 +1,28 @@
 #!/bin/bash
 
-# has to be executed from the root of the repository
+# Has to be executed from the root of the repository.
+# Usually invoked by `make test`.
 export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:./
 
+RET=0
+
+# Running the tests.
 bin/sql_grammar_test -f "test/lib/valid_queries.sql"
-RET1=$?
+RET=$(($RET + $?))
 
 bin/sql_tests
-RET2=$?
+RET=$(($RET + $?))
 
-if [[ $RET1 != 0 ]]; then exit $RET1; fi
-if [[ $RET2 != 0 ]]; then exit $RET2; fi
+# Running memory leak checks.
+echo ""
+echo "Running memory leak checks..."
 
-exit 0
+valgrind --leak-check=full --error-exitcode=1 \
+  ./bin/sql_grammar_test  -f "test/lib/valid_queries.sql" >> /dev/null
+RET=$(($RET + $?))
+
+valgrind --leak-check=full --error-exitcode=1 \
+  ./bin/sql_tests  -f "test/lib/valid_queries.sql" >> /dev/null
+RET=$(($RET + $?))
+
+exit $RET