Memory leaks in tags and NULLGUIFactory

[tylermorganwall]

Valgrind detected two memory leaks on my tests involving rgl. Here's the output of valgrind for the first:

==3657== 41 bytes in 4 blocks are definitely lost in loss record 62 of 3,245
==3657==    at 0x4844723: operator new[](unsigned long) (vg_replace_malloc.c:725)
==3657==    by 0x32B9A96A: rgl_material (api.cpp:1033)
==3657==    by 0x495B3D8: do_dotCode (dotcode.c:1972)
==3657==    by 0x4998763: bcEval_loop (eval.c:8122)
==3657==    by 0x49A5242: bcEval (eval.c:7505)
==3657==    by 0x49A5242: bcEval (eval.c:7490)
==3657==    by 0x49A55DA: Rf_eval (eval.c:1167)
==3657==    by 0x49A772E: R_execClosure (eval.c:2393)
==3657==    by 0x49A84C6: applyClosure_core (eval.c:2306)
==3657==    by 0x49A5705: Rf_applyClosure (eval.c:2328)
==3657==    by 0x49A5705: Rf_eval (eval.c:1280)
==3657==    by 0x49214FD: do_docall (coerce.c:2764)
==3657==    by 0x4998763: bcEval_loop (eval.c:8122)
==3657==    by 0x49A5242: bcEval (eval.c:7505)
==3657==    by 0x49A5242: bcEval (eval.c:7490)

The leak is here:

rgl/src/api.cpp

Lines 1031 to 1038 in ca6db5c

	size_t len_tag = strlen(cdata[0]);
	if (len_tag) {
	char* in_tag = new char [len_tag + 1];
	strncpy(in_tag, cdata[0], len_tag);
	in_tag[len_tag] = '\0';
	mat.tag = std::string(in_tag);
	} else
	mat.tag = std::string();

Specifically, char* in_tag is never deleted. This should be deleted after mat.tag is initialized. std::string copies the raw character array during initialization, so in_tag should not be needed after that.

  size_t len_tag = strlen(cdata[0]);
  if (len_tag) {
    char* in_tag = new char [len_tag + 1];
    strncpy(in_tag, cdata[0], len_tag);
    in_tag[len_tag] = '\0';
    mat.tag = std::string(in_tag);
    delete[] in_tag;
  }

The second is valgrind issue is below:

==3657== 298 (56 direct, 242 indirect) bytes in 1 blocks are definitely lost in loss record 231 of 3,245
==3657==    at 0x4842F95: operator new(unsigned long) (vg_replace_malloc.c:483)
==3657==    by 0x32BA8736: rgl::NULLGUIFactory::createWindowImpl(rgl::Window*) (NULLgui.cpp:118)
==3657==    by 0x32BA7409: rgl::Window::Window(rgl::View*, rgl::GUIFactory*) (gui.cpp:162)
==3657==    by 0x32BA1982: rgl::Device::Device(int, bool) (device.cpp:16)
==3657==    by 0x32BA1C29: rgl::DeviceManager::openDevice(bool) (devicemanager.cpp:37)
==3657==    by 0x32B98897: rgl_dev_open (api.cpp:65)
==3657==    by 0x495B8FD: do_dotCode (dotcode.c:1966)
==3657==    by 0x4998763: bcEval_loop (eval.c:8122)
==3657==    by 0x49A5242: bcEval (eval.c:7505)
==3657==    by 0x49A5242: bcEval (eval.c:7490)
==3657==    by 0x49A55DA: Rf_eval (eval.c:1167)
==3657==    by 0x49A772E: R_execClosure (eval.c:2393)
==3657==    by 0x49A84C6: applyClosure_core (eval.c:2306)

This is located here:

rgl/src/NULLgui.cpp

Lines 116 to 120 in ca6db5c

	WindowImpl* NULLGUIFactory::createWindowImpl(Window* in_window)
	{
	NULLWindowImpl* impl = new NULLWindowImpl(in_window);
	return impl;
	}

I believe the impl pointer should be deleted when the View (for the window) is destroyed here:

rgl/src/gui.cpp

Lines 57 to 64 in ca6db5c

	View::~View()
	{
	if ((windowImpl) && (flags & WINDOW_IMPL_OWNER)) {
	windowImpl->unbind();
	windowImpl->destroy();
	windowImpl = 0;
	}
	}

The windowImpl destroys its children, but the windowImpl pointer itself is never freed anywhere. I'm not so sure about the intended ownership of the windowImpl pointer, but my guess is it should be freed here.

View::~View()
{
  if ((windowImpl) && (flags & WINDOW_IMPL_OWNER)) {
    windowImpl->unbind();
    windowImpl->destroy();
    delete windowImpl;
  }
}

Note that there may also be leaks in the other (windows/X11) GUI factories as well--my valgrind tests only run on a headless system, so these are the only ones I can detect.