← Back to team overview

maria-developers team mailing list archive

Re: dbb62d20f36: Fixes that enables my_new.cc (new wrapper using my_malloc)

 

Hi, Michael!

On Mar 28, Michael Widenius wrote:
> On Sun, Mar 28, 2021 at 3:00 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> > On Mar 28, Michael Widenius wrote:
> > > revision-id: dbb62d20f36 (mariadb-10.5.2-518-gdbb62d20f36)
> > > parent(s): 0c37211c2a2
> > > author: Michael Widenius <michael.widenius@xxxxxxxxx>
> > > committer: Michael Widenius <michael.widenius@xxxxxxxxx>
> > > 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.
> 
> The problem is that one cannot and should never use HAVE_CXX_NEW as
> this is something that is found by cmake.

That's okay, it's always possible to override cmake check, cmake won't
mind. We routinely do it all the time.

You might've missed that I wrote above that I tested it and it worked.

> One cannot fix this by reseting HAVE_CXX_NEW, because my_new.cc
> requires this to be defined

No, it doesn't. There's no code that checks for HAVE_CXX_NEW,
HAVE_CXX_NEW is only for cmake and my_new.cc only checks USE_MYSYS_NEW.

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx


Follow ups

References