From: Date: October 24 2008 4:31pm Subject: bzr commit into mysql-5.1 branch (msvensson:2708) Bug#38662 List-Archive: http://lists.mysql.com/commits/57018 X-Bug: 38662 Message-Id: <20081024143146.252E930E84B@pilot> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit #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 + +#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 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 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.%