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. 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 | |
25 | void build_simple_doc(void); |
26 | int dump_attribs_to_stdout(TiXmlElement *); |
27 | void dump_to_stdout(const char *); |
28 | void dump_to_stdout(TiXmlDocument *); |
29 | void write_app_settings_doc(void); |
30 | void write_simple_doc2(void); |
31 | |
32 | const char * const APP_SETTINGS = "appsettings.xml"; |
33 | const char * const EXAMPLE1 = "example1.xml"; |
34 | const char * const EXAMPLE2 = "example2.xml"; |
35 | const char * const EXAMPLE3 = "example3.xml"; |
36 | const char * const EXAMPLE4 = "example4.xml"; |
37 | const char * const MADE_BY_HAND = "madeByHand.xml"; |
38 | const char * const MADE_BY_HAND2 = "madeByHand2.xml"; |
39 | |
40 | int 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 | |
74 | void 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 | |
100 | cleanup: |
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 |
108 | int 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 | |
142 | void 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 | |
157 | void dump_to_stdout(TiXmlNode *pParent) |
158 | { |
159 | if(!pParent) |
160 | return; |
161 | |
162 | /* Not yet implemented... */ |
163 | } |
164 | |
165 | void 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 | |
234 | void 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());
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).
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.
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.
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.
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.
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...).
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.
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.
..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).
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.
I don't see the difference (aside from requiring the semi-colon in yours).
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.
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.
An if statement is more costly than an int/pointer assignment.
I suppose that makes sense...
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.
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.
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.
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; }
References work with pointers...
OMG! 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.
I'm getting a linker error:
undefined reference to `void bam_delete<TiXmlElement>(TiXmlElement*&)'
Am I calling it wrongly?
Don't you need to specialize template functions?
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).
Then, shouldn't you call it bam_delete(window); directly?
Then, shouldn't you call it bam_delete(window); directly?
I get the same result. I don't think that matters.
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?
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)... 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.
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. Don't ask me what is or isn't compatible with Windows.
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 | |
3 | template<typename T> |
4 | inline void bam_delete(T *&ptr) |
5 | { |
6 | delete ptr; |
7 | ptr = 0; |
8 | } |
9 | |
10 | template<typename T> |
11 | inline void bam_free(T *&ptr) |
12 | { |
13 | free(ptr); |
14 | ptr = 0; |
15 | } |
src/main.cpp:
#include <bam/bam.hpp> . . . bam_delete(window);
Makefile:
1 | CPPFLAGS := -g -Iinclude |
2 | |
3 | tutorial: 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 | |
6 | obj/tinystr.o: src/tinystr.cpp |
7 | g++ $(CPPFLAGS) $^ -c -o $@ |
8 | |
9 | obj/tinyxml.o: src/tinyxml.cpp |
10 | g++ $(CPPFLAGS) $^ -c -o $@ |
11 | |
12 | obj/tinyxmlerror.o: src/tinyxmlerror.cpp |
13 | g++ $(CPPFLAGS) $^ -c -o $@ |
14 | |
15 | obj/tinyxmlparser.o: src/tinyxmlparser.cpp |
16 | g++ $(CPPFLAGS) $^ -c -o $@ |
17 | |
18 | obj/bam/bam.o: src/bam/bam.cpp |
19 | g++ $(CPPFLAGS) $^ -c -o $@ |
20 | |
21 | obj/bam/bam.c.o: src/bam/bam.c |
22 | g++ $(CPPFLAGS) $^ -c -o $@ |
23 | |
24 | obj/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++.
templates have to be inlined, and that means they have to go in their own headers.
templates have to be inlined, and that means they have to go in their own headers.
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.
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.
The do { stuff } while (0) approach though will generate less efficient code on some compilers.
Name names?
IIRC a better approach is
That's too hard to remember.
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.
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.
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.
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. I guess touch'ing the headers isn't enough. Thanks everyone! \o/
If your makefile doesn't consider headers (for example, file.o : file.cpp file.h as rule) it won't care about the headers.
If your makefile doesn't consider headers (for example, file.o : file.cpp file.h as rule) it won't care about the headers.
Ah, that never occurred to me.