[vtk-developers] DebugLeaks C++ expert issue?

[Ben Boeckel]

On Mon, Jan 07, 2019 at 11:39:17 -0500, Ken Martin via vtk-developers wrote:
> On VS2017 Debug with DEBUG_LEAKS I'm seeing an odd issue. DebugLeaks is
> complaining about vtkCommands being leaked because the two string literals
> in the below code have different addresses (the string address is used in a
> map type structure). Looking at DebugLeaks the only way this code would
> work is if they have the same address. But I checked in the debugger and
> they have different addresses.
> 
> <snip>
> 
> I can fix it by doing the following but wanted a c++ expert to weigh in,
> was the original code OK? Is this a compiler compliance issue? Is the
> proposed fix the right approach?

I don't think the standard says anything about a string literal with the
same content as another literal having to share an address. I'd expect
that this is mainly optimization at the compiler or linker level. For
example, this:

    const char* foo = "foobar";
    const char* bar = "bar";

can be satisfied with this in the resulting binary:

    "foobar\0"
     ^  ^
     |  bar
     foo

but I'd expect the standard to say that doing `foo < bar` is always
undefined behavior since they're not pointers to the same object.

> const char *cname = "vtkCommand or subclass";
> 
> //----------------------------------------------------------------
> vtkCommand::vtkCommand():AbortFlag(0),PassiveObserver(0)
> {
> #ifdef VTK_DEBUG_LEAKS
> vtkDebugLeaks::ConstructClass(cname);
> #endif
> }
> 
> //----------------------------------------------------------------
> void vtkCommand::UnRegister()
> {
> int refcount = this->GetReferenceCount()-1;
> this->SetReferenceCount(refcount);
> if (refcount <= 0)
> {
> #ifdef VTK_DEBUG_LEAKS
> vtkDebugLeaks::DestructClass(cname);
> #endif
> delete this;
> }
> }

This looks reasonable to me. Maybe add `static`?

--Ben