bzr commit into mysql-5.1 branch (msvensson:2708) Bug#38662

[Magnus Svensson]

#At file:///home/msvensson/mysql/6.2-bug38662/

 2708 Magnus Svensson	2008-10-24
      Bug#38662 Possible memleak in BasesString::assign
       - Fix the meory leak + two other instances found
       - Update the test programs for BaseString to be built and run automatically by
"make test-unit"
added:
  storage/ndb/config/type_taptest.mk.am
modified:
  storage/ndb/src/common/util/BaseString.cpp
  storage/ndb/src/common/util/Makefile.am

per-file messages:
  storage/ndb/config/type_taptest.mk.am
    Small file to "tap enable" a directory where we want to build unit tests
  storage/ndb/src/common/util/BaseString.cpp
    Fix three small bugs
     - Missing return for s==NULL path in "BaseString(const char*)"
     - Missing "delete [] m_chr" in "assign(const char*)"
     - Missing protection against appending NULL in "append(const char*)"
    Update unitests to use tap
  storage/ndb/src/common/util/Makefile.am
    Remove old testBitmask and ndb_show_compat build 
    Add build of BaseString-t unittest for BaseString
=== added file 'storage/ndb/config/type_taptest.mk.am'
--- a/storage/ndb/config/type_taptest.mk.am	1970-01-01 00:00:00 +0000
+++ b/storage/ndb/config/type_taptest.mk.am	2008-10-24 14:31:35 +0000
@@ -0,0 +1,4 @@
+INCLUDES += -I $(top_srcdir)/unittest/mytap/
+
+TAP_LIBS = $(top_builddir)/mysys/libmysyslt.la \
+	$(top_builddir)/unittest/mytap/libmytap.a

=== modified file 'storage/ndb/src/common/util/BaseString.cpp'
--- a/storage/ndb/src/common/util/BaseString.cpp	2007-04-11 13:51:09 +0000
+++ b/storage/ndb/src/common/util/BaseString.cpp	2008-10-24 14:31:35 +0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 2003 MySQL AB
+/* Copyright (C) 2003-2008 MySQL AB, 2008 Sun Microsystems Inc
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -37,6 +37,7 @@ BaseString::BaseString(const char* s)
     {
       m_chr = NULL;
       m_len = 0;
+      return;
     }
     const size_t n = strlen(s);
     m_chr = new char[n + 1];
@@ -83,6 +84,7 @@ BaseString::assign(const char* s)
 {
     if (s == NULL)
     {
+      delete[] m_chr;
       m_chr = NULL;
       m_len = 0;
       return *this;
@@ -135,6 +137,10 @@ BaseString::assign(const BaseString& str
 BaseString&
 BaseString::append(const char* s)
 {
+    if (s == NULL)
+    {
+      return *this;
+    }
     size_t n = strlen(s);
     char* t = new char[m_len + n + 1];
     if (t)
@@ -473,20 +479,20 @@ BaseString::snprintf(char *str, size_t s
 
 #ifdef TEST_BASE_STRING
 
-/*
-g++ -g -Wall -o tbs -DTEST_BASE_STRING -I$NDB_TOP/include/util \
-        -I$NDB_TOP/include/portlib BaseString.cpp
-valgrind ./tbs
-*/
+#include <tap.h>
+
+#define check(v) ok((v), " %s:%d", __FILE__, __LINE__);
 
 int main()
 {
+    plan(NO_PLAN);
+
     BaseString s("abc");
     BaseString t(s);
     s.assign("def");
     t.append("123");
-    assert(s == "def");
-    assert(t == "abc123");
+    check(s == "def");
+    check(t == "abc123");
     s.assign("");
     t.assign("");
     for (unsigned i = 0; i < 1000; i++) {
@@ -498,52 +504,68 @@ int main()
     {
 	BaseString s(":123:abc:;:foo:");
 	Vector<BaseString> v;
-	assert(s.split(v, ":;") == 7);
+	check(s.split(v, ":;") == 7);
 
-	assert(v[0] == "");
-	assert(v[1] == "123");
-	assert(v[2] == "abc");
-	assert(v[3] == "");
-	assert(v[4] == "");
-	assert(v[5] == "foo");
-	assert(v[6] == "");
+	check(v[0] == "");
+	check(v[1] == "123");
+	check(v[2] == "abc");
+	check(v[3] == "");
+	check(v[4] == "");
+	check(v[5] == "foo");
+	check(v[6] == "");
     }
 
     {
 	BaseString s(":123:abc:foo:bar");
 	Vector<BaseString> v;
-	assert(s.split(v, ":;", 4) == 4);
+	check(s.split(v, ":;", 4) == 4);
 
-	assert(v[0] == "");
-	assert(v[1] == "123");
-	assert(v[2] == "abc");
-	assert(v[3] == "foo:bar");
+	check(v[0] == "");
+	check(v[1] == "123");
+	check(v[2] == "abc");
+	check(v[3] == "foo:bar");
 
 	BaseString n;
 	n.append(v, "()");
-	assert(n == "()123()abc()foo:bar");
+	check(n == "()123()abc()foo:bar");
 	n = "";
 	n.append(v);
-	assert(n == " 123 abc foo:bar");
+	check(n == " 123 abc foo:bar");
     }
 
     {
-	assert(BaseString("hamburger").substr(4,2) == "");
-	assert(BaseString("hamburger").substr(3) == "burger");
-	assert(BaseString("hamburger").substr(4,8) == "urge");
-	assert(BaseString("smiles").substr(1,5) == "mile");
-	assert(BaseString("012345").indexOf('2') == 2);
-	assert(BaseString("hej").indexOf('X') == -1);
+	check(BaseString("hamburger").substr(4,2) == "");
+	check(BaseString("hamburger").substr(3) == "burger");
+	check(BaseString("hamburger").substr(4,8) == "urge");
+	check(BaseString("smiles").substr(1,5) == "mile");
+	check(BaseString("012345").indexOf('2') == 2);
+	check(BaseString("hej").indexOf('X') == -1);
     }
 
     {
-	assert(BaseString(" 1").trim(" ") == "1");
-	assert(BaseString("1 ").trim(" ") == "1");
-	assert(BaseString(" 1 ").trim(" ") == "1");
-	assert(BaseString("abc\t\n\r kalleabc\t\r\n").trim("abc\t\r\n ") == "kalle");
-	assert(BaseString(" ").trim(" ") == "");
+	check(BaseString(" 1").trim(" ") == "1");
+	check(BaseString("1 ").trim(" ") == "1");
+	check(BaseString(" 1 ").trim(" ") == "1");
+	check(BaseString("abc\t\n\r kalleabc\t\r\n").trim("abc\t\r\n ") == "kalle");
+	check(BaseString(" ").trim(" ") == "");
     }
-    return 0;
+
+    // Tests for BUG#38662
+    BaseString s2(NULL);
+    BaseString s3;
+    BaseString s4("elf");
+
+    ok(s3.append((const char*)NULL) == "", "append NULL");
+    ok(s4.append((const char*)NULL) == "elf", "append NULL");
+    ok(s4.append(s3) == "elf", "append emtpy");
+    ok(s4.append(s2) == "elf", "append NULL initialized");
+    ok(s4.append(s4) == "elfelf", "append self");
+
+    ok(s3.assign((const char*)NULL).c_str() == NULL, "assign NULL");
+    ok(s4.assign((const char*)NULL).c_str() == NULL, "assign NULL");
+    ok(s4.assign(s4).c_str() == NULL, "assign NULL to NULL");
+
+    return exit_status();
 }
 
 #endif

=== modified file 'storage/ndb/src/common/util/Makefile.am'
--- a/storage/ndb/src/common/util/Makefile.am	2008-08-08 05:18:53 +0000
+++ b/storage/ndb/src/common/util/Makefile.am	2008-10-24 14:31:35 +0000
@@ -27,33 +27,15 @@ libgeneral_la_SOURCES = \
             Bitmask.cpp \
 	    ndb_rand.c
 
-EXTRA_PROGRAMS = testBitmask
-testBitmask_SOURCES =  testBitmask.cpp
-testBitmask_LDFLAGS = @ndb_bin_am_ldflags@ \
-  $(top_builddir)/storage/ndb/src/libndbclient.la \
-         $(top_builddir)/dbug/libdbuglt.la \
-         $(top_builddir)/mysys/libmysyslt.la \
-         $(top_builddir)/strings/libmystringslt.la
-
-#noinst_PROGRAMS = ndb_show_compat
-#ndb_show_compat_SOURCES = ndb_show_compat.cpp
-#ndb_show_compat_LDADD = \
-#	$(top_builddir)/storage/ndb/src/common/util/libgeneral.la \
-#	$(top_builddir)/storage/ndb/src/common/portlib/libportlib.la \
-#	$(top_builddir)/dbug/libdbuglt.la \
-#	$(top_builddir)/mysys/libmysyslt.la \
-#	$(top_builddir)/strings/libmystringslt.la
-
-testBitmask.cpp : Bitmask.cpp
-	rm -f testBitmask.cpp
-	@LN_CP_F@ Bitmask.cpp testBitmask.cpp
-
-testBitmask.o:	$(testBitmask_SOURCES)
-		$(CXXCOMPILE) -c $(INCLUDES) -D__TEST_BITMASK__ $<
+noinst_PROGRAMS = BaseString-t
 
+BaseString_t_SOURCES = BaseString.cpp
+BaseString_t_CXXFLAGS = -DTEST_BASE_STRING
+BaseString_t_LDADD = libgeneral.la $(TAP_LIBS)
 
 include $(top_srcdir)/storage/ndb/config/common.mk.am
 include $(top_srcdir)/storage/ndb/config/type_util.mk.am
+include $(top_srcdir)/storage/ndb/config/type_taptest.mk.am
 
 # Don't update the files from bitkeeper
 %::SCCS/s.%