Memory leaks with my XML code?

General JUCE discussion
1

** I posted this a week or so ago, but I realized I poorly labeled the Post title. Sorry for the repost, but I’m hoping that now that I’ve properly labeled it, that I might get a few comments. **

I just finished writing what might be the ugliest piece of code I’ve ever written… but it outputs what I want (Brute force right now to get the idea working). It’s leaky, using all sorts of stuff I shouldn’t, and generally should be much nicer than it is.

The function is supposed to take an XML file that contains timeStamps (plus msg specific data) for Start, Beats, Bars, MIDI notes, and CC msgs, and then sample/quantize the MIDI notes and CC msgs at 100 hrz (leaving the beats & bars where they are). The MIDI Notes and CC msgs can come from multiple devices, and its important that each MIDI message keeps the name of the device it originated from.

I hope I’m not out of place asking for people to take a look and give me some pointers* :slight_smile: I am fully expecting a few comments on my lack of understanding with C++.

Thanks in advance for any help, I’ve tried my best to label everything with comments.

[code]void PerformerTab::downSampleData(XmlElement* MIDILog_)
{
// window increment in ms (0.001 = 1 ms)
double windowInc = 0.010;

// init the first window timeStamp by adding windowInc to our start time
double currentWindow = MIDILog_->getChildByName(TRANS("Start") )->getDoubleAttribute(TRANS("TS")) + windowInc;

bool addNewChild = true;

XmlElement xmlWindow = new XmlElement(TRANS("xmlWindow") );
String pName = performerName.replaceCharacter(' ', '_');

XmlElement* writeOutXML = new XmlElement(pName);

forEachXmlChildElement (*MIDILog_, child)
{
	// are we inside our current window
	if(child->getDoubleAttribute(TRANS("TS") ) <= currentWindow)
	{
		// is this Child a midi or cc message
	    if(child->hasAttribute(TRANS("Device")) )		    
		{
			// is this a midi note
			if(child->hasAttribute(TRANS("NN")) )
			{
				// for our list of events...
				forEachXmlChildElement (*xmlWindow, windowChild)
				{
					// does the child have a MidiNote
					if(windowChild->hasAttribute(TRANS("NN")) )
					{
						// do we already have this midi note
						if(windowChild->getIntAttribute(TRANS("NN")) == child->getIntAttribute(TRANS("NN")) )
						{	
							// does this message belong to this device or another device
							if(windowChild->getStringAttribute(TRANS("Device")) == child->getStringAttribute(TRANS("Device")) )
							{
								// add the note to exisitng value and inc mean div
								windowChild->setAttribute(TRANS("Vel"), (windowChild->getIntAttribute(TRANS("Vel")) + child->getIntAttribute(TRANS("Vel"))) );
								windowChild->setAttribute(TRANS("mean_div"), (windowChild->getIntAttribute(TRANS("mean_div")) + 1) );
								
								addNewChild = false;
							}
						}
					}
				}
				
				// we didn't find the device and note number so create a new event
				if(addNewChild)
				{
					// create new midi event element
					XmlElement* newEvent = new XmlElement(TRANS("MidiEvent") );
					newEvent->setAttribute(TRANS("Device"), child->getStringAttribute(TRANS("Device")) );
					newEvent->setAttribute(TRANS("NN"), child->getIntAttribute(TRANS("NN")) );
					newEvent->setAttribute(TRANS("Vel"), child->getIntAttribute(TRANS("Vel")) );
					newEvent->setAttribute(TRANS("mean_div"), 1 );
					xmlWindow->addChildElement(newEvent);
				}
				
				addNewChild = true;
			}
			// is this a CC msg
			else if(child->hasAttribute(TRANS("CCN")) )
			{
				// for our list of events...
				forEachXmlChildElement (*xmlWindow, windowChild)
				{
					// does the child have a CC msg
					if(windowChild->hasAttribute(TRANS("CCN")) )
					{
						// do we already have this CC msg
						if(windowChild->getIntAttribute(TRANS("CCN")) == child->getIntAttribute(TRANS("CCN")) )
						{
							// does this message belong to this device or another device
							if(windowChild->getStringAttribute(TRANS("Device")) == child->getStringAttribute(TRANS("Device")) )
							{
								// add the note to exisitng value and inc mean div
								windowChild->setAttribute(TRANS("cc_Vel"), (windowChild->getIntAttribute(TRANS("cc_Vel")) + child->getIntAttribute(TRANS("cc_Vel"))) );
								windowChild->setAttribute(TRANS("mean_div"), (windowChild->getIntAttribute(TRANS("mean_div")) + 1) );
								
								addNewChild = false;
							}
						}
					}
				}
				
				// we didn't find the device and CC number so create a new event
				if(addNewChild)
				{
					// create new CC event element
					XmlElement* newEvent = new XmlElement(TRANS("CCEvent") );
					newEvent->setAttribute(TRANS("Device"), child->getStringAttribute(TRANS("Device")) );
					newEvent->setAttribute(TRANS("CCN"), child->getIntAttribute(TRANS("CCN")) );
					newEvent->setAttribute(TRANS("cc_Vel"), child->getIntAttribute(TRANS("cc_Vel")) );
					newEvent->setAttribute(TRANS("mean_div"), 1 );
					xmlWindow->addChildElement(newEvent);
				}
				
				addNewChild = true;
			}
		}
		else
		{
			// this is a beat tag so add it to the new xmlfile as is
			XmlElement* temp = new XmlElement(*child);
			writeOutXML->addChildElement(temp);
		}
	}
	else
	{
		// we finished a window so lets filter and add the new XmlChildElements
		forEachXmlChildElement (*xmlWindow, windowChild)
		{ 
			// does the child have a MIDI note
			if(windowChild->hasAttribute(TRANS("NN")) )
			{
				windowChild->setAttribute(TRANS("Vel"), (windowChild->getIntAttribute(TRANS("Vel")) / windowChild->getIntAttribute(TRANS("mean_div"))) );
				
				XmlElement* newEvent = new XmlElement(TRANS("MIDI_Note") );
				newEvent->setAttribute(TRANS("Device"), windowChild->getStringAttribute(TRANS("Device")) );
				newEvent->setAttribute(TRANS("NN"), windowChild->getIntAttribute(TRANS("NN")) );
				newEvent->setAttribute(TRANS("Vel"), windowChild->getIntAttribute(TRANS("Vel")) );
				newEvent->setAttribute(TRANS("TS"), currentWindow );
				writeOutXML->addChildElement(newEvent);
			}
			
			// does the child have a CC msg
			else if(windowChild->hasAttribute(TRANS("CCN")) )
			{
				windowChild->setAttribute(TRANS("cc_Vel"), (windowChild->getIntAttribute(TRANS("cc_Vel")) / windowChild->getIntAttribute(TRANS("mean_div"))) );
				
				XmlElement* newEvent = new XmlElement(TRANS("CC_Message") );
				newEvent->setAttribute(TRANS("Device"), windowChild->getStringAttribute(TRANS("Device")) );
				newEvent->setAttribute(TRANS("CCN"), windowChild->getIntAttribute(TRANS("CCN")) );
				newEvent->setAttribute(TRANS("cc_Vel"), windowChild->getIntAttribute(TRANS("cc_Vel")) );
				newEvent->setAttribute(TRANS("TS"), currentWindow );
				writeOutXML->addChildElement(newEvent);
			}
		}
		
		// now that we've added the window of XmlChildElements, lets clear the Xml window file
		xmlWindow->deleteAllChildElements();
		
		// if this is a MIDI or CC event then this is the first event of a new window
		if(child->hasAttribute(TRANS("Device")) )
		{
			// is this a midi note
			if(child->hasAttribute(TRANS("NN")) )
			{
				// create new midi event element
				XmlElement* newEvent = new XmlElement(TRANS("MidiEvent") );
				newEvent->setAttribute(TRANS("Device"), child->getStringAttribute(TRANS("Device")) );
				newEvent->setAttribute(TRANS("NN"), child->getIntAttribute(TRANS("NN")) );
				newEvent->setAttribute(TRANS("Vel"), child->getIntAttribute(TRANS("Vel")) );
				newEvent->setAttribute(TRANS("mean_div"), 1 );
				xmlWindow->addChildElement(newEvent);
			}
			// is this a CC msg
			else if(child->hasAttribute(TRANS("CCN")) )
			{
				// create new CC event element
				XmlElement* newEvent = new XmlElement(TRANS("CCEvent") );
				newEvent->setAttribute(TRANS("Device"), child->getStringAttribute(TRANS("Device")) );
				newEvent->setAttribute(TRANS("CCN"), child->getIntAttribute(TRANS("CCN")) );
				newEvent->setAttribute(TRANS("cc_Vel"), child->getIntAttribute(TRANS("cc_Vel")) );
				newEvent->setAttribute(TRANS("mean_div"), 1 );
				xmlWindow->addChildElement(newEvent);
			}
		}
		else
		{
			// this is a beat tag so add it to the new xmlfile as is
			XmlElement* temp = new XmlElement(*child);
			writeOutXML->addChildElement(temp);
		}
		
		// make sure our current window will contain this new event
		while(currentWindow < child->getDoubleAttribute(TRANS("TS")) )
			currentWindow += windowInc;
	}
}

// delete all the old XmlElements
MIDILog_->deleteAllChildElements();

// add the new filtered XmlElements
forEachXmlChildElement(*writeOutXML, child)
{
	XmlElement* temp = new XmlElement(*child);
	MIDILog_->addChildElement(temp);
}

}[/code]

2

For anyone interested, I switched all my XmlElement pointers to ScopedPointers and all my leaks disappeared.

1 Like
3

Yeah, you’ve got some pretty weird-looking stuff going on in there… Can’t imagine why you’ve use the TRANS macro like that, translating XML tags looks like a dodgy idea to me. Just stick to normal strings - even the T macro is usually unnecessary these days.

4

Thanks for the tip, I was writing that code as I was learning C++ and started by using String::String(“xxxx”). A look through the forums suggested always using T… which then I saw turned into a Char array, so then I saw you use TRANS in some of your code, which seemed to give a constant string of some kind… So I figured I’d use that.

Lately I’ve just been using String(“xxxxx”) to pass in the strings. Seems to work, but I’m still unclear what to use when?

5

Surely if you look at any of my code you’ll see it all just uses plain string literals, “xxxx”… The cast to string is almost always implicit.

6

That’ s good to know. so anytime a JUCE function wants a string I should just use either

juceObject.juceFunctionThatTakesAString(“xxxxx” );

of if I need to convert a numeric value to a string

in j = 10;

juceObject.juceFunctionThatTakesAString(String(j) );

7

Yup.