Hi, Michael! On Mar 28, Michael Widenius wrote: > revision-id: dbb62d20f36 (mariadb-10.5.2-518-gdbb62d20f36) > parent(s): 0c37211c2a2 > author: Michael Widenius <michael.widen...@gmail.com> > committer: Michael Widenius <michael.widen...@gmail.com> > timestamp: 2021-03-24 14:31:53 +0200 > message: > > Fixes that enables my_new.cc (new wrapper using my_malloc) > > This is not enabled by default as there are leaks in the > server that needs to be fixed first. One can compile > with -DREALLY_USE_MYSYS_NEW -DSF_REMEMBER_FRAMES=14 to find the > memory leaks from 'new'
The change in my_new.cc is ok. The change in my_global.h and CMakeLists.txt is confusing and redundant. You unconditionaly define USE_MYSYS_NEW then conditionally undefine it again? Everything already worked just fine. By default cxx new is used, if one wants to use mysys version, it's done with -DHAVE_CXX_NEW=0 this works for me in 10.5 and fails to compile my_new.cc, as it should. Please, only keed your my_new.cc changes and say in the commit message "One can compile with -DHAVE_CXX_NEW=0 to find memory leaks from 'new'" As for -DSF_REMEMBER_FRAMES=14, perhaps it's better to do it automatically? Like #ifdef USE_MYSYS_NEW #define SF_REMEMBER_FRAMES 14 #else #define SF_REMEMBER_FRAMES 8 #endif or may be even just always make it 14. It should be easy to enable this feature, right? > diff --git a/CMakeLists.txt b/CMakeLists.txt > index a0b540f12d2..c51fb98abdc 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -366,10 +366,8 @@ ENDIF() > # Run platform tests > INCLUDE(configure.cmake) > > -# force -DUSE_MYSYS_NEW unless already done by HAVE_CXX_NEW > -IF(NOT HAVE_CXX_NEW) > - ADD_DEFINITIONS(-DUSE_MYSYS_NEW) > -ENDIF() > +# Always use USE_MYSYS_NEW > +ADD_DEFINITIONS(-DUSE_MYSYS_NEW) > > # Find header files from the bundled libraries > # (wolfssl, readline, pcre2, etc) > diff --git a/include/my_global.h b/include/my_global.h > index e999e555bf7..6e3b48a00fd 100644 > --- a/include/my_global.h > +++ b/include/my_global.h > @@ -487,6 +487,9 @@ typedef unsigned short ushort; > duplicate declaration of __cxa_pure_virtual, solved by declaring it a > weak symbol. > */ > +#ifndef REALLY_USE_MYSYS_NEW > +#undef USE_MYSYS_NEW > +#endif > #if defined(USE_MYSYS_NEW) && ! defined(DONT_DECLARE_CXA_PURE_VIRTUAL) > C_MODE_START > int __cxa_pure_virtual () __attribute__ ((weak)); > diff --git a/mysys/my_static.c b/mysys/my_static.c > index 3c4b9efc1f8..96a23cdb283 100644 > --- a/mysys/my_static.c > +++ b/mysys/my_static.c > @@ -48,6 +48,7 @@ PSI_memory_key key_memory_my_err_head; > PSI_memory_key key_memory_my_file_info; > PSI_memory_key key_memory_pack_frm; > PSI_memory_key key_memory_charsets; > +PSI_memory_key key_memory_new; where do you initialize it? I'd suggest statically, like PSI_memory_key key_memory_new= PSI_INSTRUMENT_MEM; > > #ifdef _WIN32 > PSI_memory_key key_memory_win_SECURITY_ATTRIBUTES; Regards, Sergei VP of MariaDB Server Engineering and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
