Allegro.cc - Online Community

Allegro.cc Forums » Programming Questions » tinyxml -- Memory Management -- Does It Manage Some Or Is It All You?

Credits go to ReyBrujo for helping out!
This thread is locked; no one can reply to it. rss feed Print
 1   2 
tinyxml -- Memory Management -- Does It Manage Some Or Is It All You?
bamccaig
Member #7,536
July 2006
avatar

OK, so I decided to try to learn some XML related stuff and figured the best place to start would be actually working with documents in a real programming language. ;) IIRC, I actually initially searched for XPath and tinyxpath caught my attention so I figured it was a good place to start. When I saw that it made use of tinyxml I decided to start with tinyxml instead.

I started by downloading tinyxml, tinyxpath, and tinyxml++ source to the computer sciences server at the college and compiled all three. I read through some tinyxml documentation and started following this tutorial.

It's not even really so much a tutorial as it is example code in unstructured order. :P Anyway, it's enough to get the point across. However, it doesn't appear that the author of the tutorial (at least as far as I am in the tutorial) is concerning himself with memory management. For example, we're instantiating numerous objects and attaching them to stack objects, but we never destroy them. In particular, we're creating TiXmlComments, TiXmlDeclarations, TiXmlElements, and TiXmlTexts; and generally LinkEndChild'ing them to a TiXmlDocument on the stack (or to other heap objects, like TiXmlElements).

I'm wondering if anybody knows whether the TiXmlDocument is going to manage these children or whether I'm supposed to destroy them after (which seems more likely). I did a little reading through the documentation and checked the source and didn't see anything to suggest that the TiXmlDocument would be managing the memory of children so I wrote a simple macro to check a pointer, delete it, and set it to 0 and started implementing it. However, for some reason one of my pointers is causing a SEGFAULT in (IIRC) build_simple_doc on line 101.
DELETE(text);
Does anybody see what might be causing this? I ran it through gdb and printed the value of text and it has a non-zero value so I assume it was allocated successfully... :-/

1#include <cstdio>
2#include "tinystr.h"
3#include "tinyxml.h"
4 
5/*** DELETE(ptr) and FREE(ptr) macros are not part of the tutorial ***/
6 
7#define DELETE(ptr) \
8 { \
9 if(ptr) \
10 { \
11 delete ptr; \
12 ptr = 0; \
13 } \
14 }
15 
16#define FREE(ptr) \
17 { \
18 if(ptr) \
19 { \
20 free(ptr); \
21 ptr = 0; \
22 } \
23 }
24 
25void build_simple_doc(void);
26int dump_attribs_to_stdout(TiXmlElement *);
27void dump_to_stdout(const char *);
28void dump_to_stdout(TiXmlDocument *);
29void write_app_settings_doc(void);
30void write_simple_doc2(void);
31 
32const char * const APP_SETTINGS = "appsettings.xml";
33const char * const EXAMPLE1 = "example1.xml";
34const char * const EXAMPLE2 = "example2.xml";
35const char * const EXAMPLE3 = "example3.xml";
36const char * const EXAMPLE4 = "example4.xml";
37const char * const MADE_BY_HAND = "madeByHand.xml";
38const char * const MADE_BY_HAND2 = "madeByHand2.xml";
39 
40int main(int arc, char* argv[])
41{
42 TiXmlDocument doc(EXAMPLE1);
43 TiXmlElement *window = NULL;
44 
45 doc.LoadFile();
46 
47 try
48 {
49 window = new TiXmlElement("Demo");
50 window->SetAttribute("name", "Circle");
51 window->SetAttribute("x", 5);
52 window->SetAttribute("y", 15);
53 window->SetDoubleAttribute("radius", 3.14159);
54 
55 dump_attribs_to_stdout(window);
56 }
57 catch(...)
58 {
59 printf("main(int, char**): An unknown error occurred.\n");
60 }
61 
62 build_simple_doc();
63 write_app_settings_doc();
64 write_simple_doc2();
65 dump_to_stdout(EXAMPLE1);
66 dump_to_stdout(MADE_BY_HAND);
67 dump_to_stdout(MADE_BY_HAND2);
68 
69 DELETE(window);
70 
71 return(0);
72}
73 
74void build_simple_doc(void)
75{
76 // Make xml: <?xml ..><Hello>World</Hello>
77 TiXmlDocument doc;
78 TiXmlDeclaration *decl = NULL;
79 TiXmlElement *element = NULL;
80 TiXmlText *text = NULL;
81 
82 try
83 {
84 decl = new TiXmlDeclaration("1.0", "", "");
85 element = new TiXmlElement("Hello");
86 text = new TiXmlText("World");
87 }
88 catch(...)
89 {
90 printf("build_simple_doc(): An unknown error occurred.\n");
91 goto cleanup;
92 }
93 
94 element->LinkEndChild(text);
95 doc.LinkEndChild(decl);
96 doc.LinkEndChild(element);
97 
98 doc.SaveFile(MADE_BY_HAND);
99 
100cleanup:
101 DELETE(decl);
102 DELETE(element);
103 DELETE(text); /**** SEGFAULT HERE ***/
104}
105 
106// print all attributes of pElement.
107// returns the number of attributes printed
108int dump_attribs_to_stdout(TiXmlElement *pElement)
109{
110 double dval;
111 int i=0;
112 int ival;
113 TiXmlAttribute *pAttrib = NULL;
114 
115 if(!pElement)
116 return(0);
117 
118 printf("\n");
119 
120 pAttrib = pElement->FirstAttribute();
121 
122 while(pAttrib)
123 {
124 printf("\t%s: value=[%s]", pAttrib->Name(), pAttrib->Value());
125 
126 if(pAttrib->QueryIntValue(&ival) == TIXML_SUCCESS)
127 printf("\tint=%d", ival);
128 
129 if(pAttrib->QueryDoubleValue(&dval) == TIXML_SUCCESS)
130 printf("\tdouble=%1.1f", dval);
131 
132 printf("\n");
133 
134 i++;
135 
136 pAttrib = pAttrib->Next();
137 }
138 
139 return(i);
140}
141 
142void dump_to_stdout(const char *pFilename)
143{
144 TiXmlDocument doc(pFilename);
145 
146 if(doc.LoadFile())
147 {
148 printf("\n%s:\n", pFilename);
149// dump_to_stdout(&doc);
150 }
151 else
152 {
153 printf("dump_to_stdout(const char *): Failed to load file \"%s\"\n", pFilename);
154 }
155}
156 
157void dump_to_stdout(TiXmlNode *pParent)
158{
159 if(!pParent)
160 return;
161 
162 /* Not yet implemented... */
163}
164 
165void write_app_settings_doc(void)
166{
167 TiXmlDocument doc;
168 TiXmlElement *msg = NULL;
169 TiXmlDeclaration *decl = NULL;
170 TiXmlElement *root = NULL;
171 TiXmlComment *comment = NULL;
172 TiXmlElement *msgs = NULL;
173 TiXmlElement *windows = NULL;
174 TiXmlElement *window = NULL;
175 TiXmlElement *cxn = NULL;
176 
177 try
178 {
179 decl = new TiXmlDeclaration("1.0", "", "");
180 doc.LinkEndChild(decl);
181 
182 root = new TiXmlElement("MyApp");
183 doc.LinkEndChild(root);
184 
185 comment = new TiXmlComment();
186 comment->SetValue(" Settings for MyApp ");
187 root->LinkEndChild(comment);
188 
189 msgs = new TiXmlElement("Messages");
190 root->LinkEndChild(msgs);
191 
192 msg = new TiXmlElement("Welcome");
193 msg->LinkEndChild(new TiXmlText("Welcome to MyApp"));
194 msgs->LinkEndChild(msg);
195 
196 msg = new TiXmlElement("Farewell");
197 msg->LinkEndChild(new TiXmlText("Thank you for using MyApp"));
198 msgs->LinkEndChild(msg);
199 
200 windows = new TiXmlElement("Windows");
201 root->LinkEndChild(windows);
202 
203 window = new TiXmlElement("Window");
204 windows->LinkEndChild(window);
205 window->SetAttribute("name", "MainFrame");
206 window->SetAttribute("x", 5);
207 window->SetAttribute("y", 15);
208 window->SetAttribute("w", 400);
209 window->SetAttribute("h", 250);
210 
211 cxn = new TiXmlElement("Connection");
212 root->LinkEndChild(cxn);
213 cxn->SetAttribute("ip", "192.168.0.1");
214 cxn->SetDoubleAttribute("timeout", 123.456); // Floating point attrib
215 
216// dump_to_stdout(&doc);
217 doc.SaveFile(APP_SETTINGS);
218 }
219 catch(...)
220 {
221 printf("write_app_settings_doc(): An unknown error occurred.\n");
222 }
223 
224 DELETE(msg);
225 DELETE(decl);
226 DELETE(root);
227 DELETE(comment);
228 DELETE(msgs);
229 DELETE(windows);
230 DELETE(window);
231 DELETE(cxn);
232}
233 
234void write_simple_doc2(void)
235{
236 // Same as build_simple_doc but add each node
237 // as early as possible into the tree.
238 
239 TiXmlDocument doc;
240 TiXmlDeclaration *decl = NULL;
241 TiXmlElement *element = NULL;
242 TiXmlText *text = NULL;
243 
244 try
245 {
246 decl = new TiXmlDeclaration("1.0", "", "");
247 doc.LinkEndChild(decl);
248 
249 element = new TiXmlElement("Hello");
250 doc.LinkEndChild(element);
251 
252 text = new TiXmlText("World");
253 element->LinkEndChild(text);
254 
255 doc.SaveFile(MADE_BY_HAND2);
256 }
257 catch(...)
258 {
259 printf("write_simple_doc2(): An unknown error occurred.\n");
260 }
261 
262 DELETE(decl);
263 DELETE(element);
264 DELETE(text);
265}

** EDIT **

I added a line just before the 3 DELETE macros where the SEGFAULT occurs and accessed a member of text and it seemingly worked fine:
printf("text->CDATA() = %d\n", text->CDATA());
:-/

ReyBrujo
Moderator
January 2001
avatar

I am guessing tinyxml frees the memory of all its children (just like any XML library would do), so you are basically double freeing the pointer.

(Edited: Indeed, it loops all the nodes and deletes them all).

--
RB
光子「あたしただ…奪う側に回ろうと思っただけよ」
Mitsuko's last words, Battle Royale

bamccaig
Member #7,536
July 2006
avatar

Ah, I see... :-/ I was looking at tinyxml.cpp from the Online Documentation and all of the destructors seem to be empty... :-/ Maybe that documentation is out of date. So I guess I don't have to worry about it while using a static parent... :-/ Thanks, ReyBrujo. :-[

Speedo
Member #9,783
May 2008

The segfault is probably not from your macro, but from the document's destructor as it goes out of scope.

When you programaticaly build a file with Tinyxml, you have two options for adding new elements:
LinkEndChild() - you pass the element a pointer (must be created by new), the element takes ownership of it and is responsible for deleting it.
InsertEndChild() - you pass the element an object and the element makes its own copy internally

I prefer the latter. It tends to avoid this confusion about "well, do I need to need to free the memory, or not...?". If you're in a situation where you're anal about objects being copied, obviously you may prefer the former method.

Personally, I would skip the tutorial (it's not very helpful) and just start working on something yourself. Tinyxml is really very easy to use once you grasp the basics.

Kitty Cat
Member #2,815
October 2002
avatar

delete and free will already no-op when given NULL. You don't need to check them. And when doing a block command with a macro, you should use do{}while(0) like:

#define MYMACRO() do { \
  ..stuff.. \ 
} while(0) 
...
MYMACRO();

..so you won't have any nasty side effects with the semicolon or if/for/while commands.

--
"Do not meddle in the affairs of cats, for they are subtle and will pee on your computer." -- Bruce Graham

bamccaig
Member #7,536
July 2006
avatar

Speedo said:

The segfault is probably not from your macro, but from the document's destructor as it goes out of scope.

Yeah, I suspected that after ReyBrujo pointed out that the document was in fact destroying it's children.

Speedo said:

I prefer the latter. It tends to avoid this confusion about "well, do I need to need to free the memory, or not...?". If you're in a situation where you're anal about objects being copied, obviously you may prefer the former method.

I like that there is a more clear approach, but I also like the efficiency of using pointers instead of forcing it to copy objects. Plus, you can theoretically reduce the number of variables required by reusing some of the pointers whereas stack objects would need to be declared individually (though I guess if they're copied they could be reused too...).

Speedo said:

Personally, I would skip the tutorial (it's not very helpful) and just start working on something yourself. Tinyxml is really very easy to use once you grasp the basics.

I suspect you're right, but I don't think there's very much left to the tutorial anyway. Might as well see what else they've done. I'll probably be trying to find a tinyxpath tutorial next.

Kitty Cat said:

delete and free will already no-op when given NULL.

I knew free did, but wasn't sure about delete and wanted to eliminate that as a possibility for causing a crash. Also, it would save the unnecessary assignment. I'm not sure which would be slower between an unnecessary condition and an unnecessary assignment. :-/

Kitty Cat said:

..so you won't have any nasty side effects with the semicolon or if/for/while commands.

I don't see the difference (aside from requiring the semi-colon in yours). :-/

Kitty Cat
Member #2,815
October 2002
avatar

Quote:

I knew free did, but wasn't sure about delete and wanted to eliminate that as a possibility for causing a crash. Also, it would save the unnecessary assignment. I'm not sure which would be slower between an unnecessary condition and an unnecessary assignment.

An if statement is more costly than an int/pointer assignment. As long as you always set a pointer to 0/NULL after you free or delete it, you never have to check before you free or delete it.

Quote:

I don't see the difference (aside from requiring the semi-colon in yours).

http://www.noveltheory.com/TechPapers/while.htm

--
"Do not meddle in the affairs of cats, for they are subtle and will pee on your computer." -- Bruce Graham

Speedo
Member #9,783
May 2008

The do { stuff } while (0) approach though will generate less efficient code on some compilers. IIRC a better approach is

#define MYMACRO if (1) \
{ \ 
somefunction(); \ 
someotherfunction(); \ 
} \ 
else (void)0 

Either way it's just another reason IMO to trash macros whenever possible and use inline functions.

Quote:

I don't see the difference (aside from requiring the semi-colon in yours).

Consider this code with your macro:

bool isDone;

...

if (isDone)
  DELETE(somepointer);
else
  cout << "not done!";

The if statement would expand to:

if (isDone)
{
  if (somepointer)
  {
    delete somepointer;
    somepointer = 0;
  }
}; // <-- BAD!
else
  cout << "not done!";

That semicolon is going to seperate the if statement from the else... and you'll get compile errors.

bamccaig
Member #7,536
July 2006
avatar

Kitty Cat said:

An if statement is more costly than an int/pointer assignment.

I suppose that makes sense... :-/

Kitty Cat said:

As long as you always set a pointer to 0/NULL after you free or delete it, you never have to check before you free or delete it.

Yes, well, as long as you free memory resources after you're done with them there will be no need for garbage collectors, but modern language designers still thought they were useful. ;) I see what you're saying though. :) Thank you.

Speedo said:

Either way it's just another reason IMO to trash macros whenever possible and use inline functions.

The problem with an inline function is that I would need to pass a pointer to my pointer to assign it a NULL value, which is less pretty (because of the address-of operator) and more error prone. A macro avoids those subtleties.

Speedo said:

That semicolon is going to seperate the if statement from the else... and you'll get compile errors.

Ah, I see... :-/ As you might have guessed, I don't use macros very often. :P

Speedo
Member #9,783
May 2008

Quote:

The problem with an inline function is that I would need to pass a pointer to my pointer to assign it a NULL value, which is less pretty (because of the address-of operator) and more error prone. A macro avoids those subtleties.

References work with pointers... ;)

template<typename T>
inline void DELETE(T*& ptr)
{
  delete ptr;
  ptr = 0;
}

bamccaig
Member #7,536
July 2006
avatar

Speedo said:

References work with pointers... ;)

OMG! :o I never thought of that! Thanks!!

** EDIT **

I'm having problems calling it though. :-/ They might be to do with my project structure or Makefile, but I'm not sure.

1template<typename T>
2inline void bam_delete(T *&ptr)
3{
4 delete ptr;
5 ptr = 0;
6}
7 
8.
9.
10.
11 
12TiXmlElement *window = new TiXmlElement("Window");
13 
14.
15.
16.
17 
18bam_delete<TiXmlElement>(window);

I'm getting a linker error:

undefined reference to `void bam_delete<TiXmlElement>(TiXmlElement*&)'

Am I calling it wrongly? :-X

ReyBrujo
Moderator
January 2001
avatar

Don't you need to specialize template functions?

--
RB
光子「あたしただ…奪う側に回ろうと思っただけよ」
Mitsuko's last words, Battle Royale

bamccaig
Member #7,536
July 2006
avatar

ReyBrujo said:

Don't you need to specialize template functions?

I don't formally know how to use templates... :( I've gotten by a little bit, but don't actually know the rules so I could easily be doing something wrong... According to this, template function specialization is used to handle specific cases that otherwise wouldn't work (i.e. a max function using the > operator won't work on char * without a specialized implementation).

ReyBrujo
Moderator
January 2001
avatar

Then, shouldn't you call it bam_delete(window); directly?

--
RB
光子「あたしただ…奪う側に回ろうと思っただけよ」
Mitsuko's last words, Battle Royale

bamccaig
Member #7,536
July 2006
avatar

ReyBrujo said:

Then, shouldn't you call it bam_delete(window); directly?

I get the same result. :-/ I don't think that matters.

Speedo
Member #9,783
May 2008

Quote:

I'm getting a linker error:
undefined reference to `void bam_delete<TiXmlElement>(TiXmlElement*&)'
Am I calling it wrongly?

Well, you don't have give the template parameter when you call it (though it shouldn't make a difference if you do). Where did you define the function, how is it included, etc?

bamccaig
Member #7,536
July 2006
avatar

That's the ugly part... I'm using a Makefile and I don't really know what I'm doing with them... :( I'll just .tar.gz the whole project directory (AFAIK, you can freely redistribute tinyxml without modifying the license information in the header)... :-X It's a small project so don't let that scare you. Let me know if you're unable to open .tar.gz, although 7-Zip for Windows can do it and is better than any other software I've used in Windows.

tinyxml-tutorial.tar.gz

The implementation is in include/bam and src/bam and everything is compiled with a very manual Makefile because I haven't figured out Makefiles yet. :P Don't ask me what is or isn't compatible with Windows. :P

src/bam/bam.hpp:

#ifndef BAM_HPP
  #define BAM_HPP

        /* bam/bam.h (and bam/bam.c) is the macro implementation... */
  #include <bam/bam.h>

template<typename T>
inline void bam_delete(T *&);

template<typename T>
inline void bam_free(T *&);

#endif

src/bam/bam.cpp:

1#include <bam/bam.hpp>
2 
3template<typename T>
4inline void bam_delete(T *&ptr)
5{
6 delete ptr;
7 ptr = 0;
8}
9 
10template<typename T>
11inline void bam_free(T *&ptr)
12{
13 free(ptr);
14 ptr = 0;
15}

src/main.cpp:

#include <bam/bam.hpp>

.
.
.

bam_delete(window);

Makefile:

1CPPFLAGS := -g -Iinclude
2 
3tutorial: obj/bam/bam.c.o obj/bam/bam.o obj/tinystr.o obj/tinyxml.o obj/tinyxmlerror.o obj/tinyxmlparser.o obj/main.o
4 g++ $(CPPFLAGS) $^ -o tutorial
5 
6obj/tinystr.o: src/tinystr.cpp
7 g++ $(CPPFLAGS) $^ -c -o $@
8 
9obj/tinyxml.o: src/tinyxml.cpp
10 g++ $(CPPFLAGS) $^ -c -o $@
11 
12obj/tinyxmlerror.o: src/tinyxmlerror.cpp
13 g++ $(CPPFLAGS) $^ -c -o $@
14 
15obj/tinyxmlparser.o: src/tinyxmlparser.cpp
16 g++ $(CPPFLAGS) $^ -c -o $@
17 
18obj/bam/bam.o: src/bam/bam.cpp
19 g++ $(CPPFLAGS) $^ -c -o $@
20 
21obj/bam/bam.c.o: src/bam/bam.c
22 g++ $(CPPFLAGS) $^ -c -o $@
23 
24obj/main.o: src/main.cpp
25 g++ $(CPPFLAGS) $^ -c -o $@

** EDIT **

Unless the #include <bam/bam.hpp> should be using double-quotes (""), even though I added a -Iinclude option to g++. :-X

Edgar Reynaldo
Major Reynaldo
May 2007
avatar

bamccaig
Member #7,536
July 2006
avatar

Edgar Reynaldo said:

templates have to be inlined, and that means they have to go in their own headers.

???

Edgar Reynaldo
Major Reynaldo
May 2007
avatar

bamccaig
Member #7,536
July 2006
avatar

Edgar Reynaldo said:

You can't link to a template class across modules - you have to completely define the template class in a header file. It has something to do with instantiation of templated objects.

I moved the template function definitions into include/bam/bam.hpp and it still doesn't work... :-/ Same linking error. :-X

Peter Wang
Member #23
April 2000

Quote:

The do { stuff } while (0) approach though will generate less efficient code on some compilers.

Name names?

Quote:

IIRC a better approach is

That's too hard to remember.

Edgar Reynaldo
Major Reynaldo
May 2007
avatar

It may not matter, but did you comment out the declarations, and just leave the definitions? You could try using template <class T> instead of template <typename T>. Don't forget -Wall in your makefile's CPPFLAGS.

Edit
It could be old .o files causing it - try cleaning them out and rebuilding it.

bamccaig
Member #7,536
July 2006
avatar

Edgar Reynaldo said:

It may not matter, but did you comment out the declarations, and just leave the definitions?

I removed the declarations and just left the definitions.

Edgar Reynaldo said:

You could try using template <class T> instead of template <typename T>. Don't forget -Wall in your makefile's CPPFLAGS.

I'll give it a try.

Edgar Reynaldo said:

It could be old .o files causing it - try cleaning them out and rebuilding it.

I touch'd all the include files before make'ing so I assume that should have rebuilt the whole thing. I'll give it a try though.

** EDIT **

No, that was it. Old .o files were getting in the way. :-X I guess touch'ing the headers isn't enough. Thanks everyone! \o/

ReyBrujo
Moderator
January 2001
avatar

If your makefile doesn't consider headers (for example, file.o : file.cpp file.h as rule) it won't care about the headers.

--
RB
光子「あたしただ…奪う側に回ろうと思っただけよ」
Mitsuko's last words, Battle Royale

 1   2 


Go to: