<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <html> <head> <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body bgcolor="#ffffff" text="#000000"> <tt>Hi, Sean Millichamp wrote the following on 04.11.2009 16:07:</tt> <blockquote cite="mid:1257347278.425.38.camel@localhost" type="cite"> <pre wrap=""><tt> Actually, api_query hasn't been initialized up until that point. The api_query pointer in your first asprintf is dependent on the results of the asprintf in your else clause. If you wanted to rewrite it then it would need to be more like: if(bare_update_check==FALSE) { asprintf(&api_query,"v=1&product=icinga&tinycheck=1&stableonly=1&version=%s%s",PROGRAM_VERSION,(api_query_opts==NULL)?"":api_query_opts); } else { asprintf(&api_query,"v=1&product=icinga&tinycheck=1&stableonly=1"); } This does cause some duplication in the strings, but does otherwise look a bit cleaner. </tt></pre> </blockquote> <tt>Ah, yeah, I knew I missed sth on that. Thanks for clearifying! Regarding the duplicated string, maybe an extra buf is needed.</tt> <blockquote cite="mid:1257347278.425.38.camel@localhost" type="cite"> <pre wrap=""><tt> I think that is a fine solution and also much cleaner looking. I was trying to keep changes to the style to a minimum, but it is often hard to best know how to do that for a given project. </tt></pre> </blockquote> <tt>To be honest, I don't really like the style of code in there. And for what it's worth some solutions are much more comfortable to implement not regarding the easier reading. That's a thing coming from rewriting IDOUtils for more RDBMs, I hesitated several times with good looking lines but not that easy code. </tt> <blockquote cite="mid:1257347278.425.38.camel@localhost" type="cite"> <pre wrap=""><tt> Which would hopefully allow the asprintf() style calls in the same fashion as they had been used, except substituting my_asprintf() for them. </tt></pre> </blockquote> <tt>asprintf is heavily used within the code so it would be a good option. Compared to how much work it would be rewriting adn aptching that, well. </tt> <blockquote cite="mid:1257347278.425.38.camel@localhost" type="cite"> <pre wrap=""><tt>And, yes, the current code does cause a small memory leak. As far as I can see, asprintf always malloc()s new memory and places the pointer in the first argument without using the existing value (no realloc, free, etc.) I found it while running valgrind on Nagios while pursuing a different problem and figured it was worthy of a janitorial patch. </tt></pre> </blockquote> <tt>Jep, asprintf allocated buffers need to be free'd manually. If you take a deeper look into e.g. NDOUtils dbhandler.c you will find lots of those statements. It's pure C but the bigger the code the more you have to correct afterwards ;-) Kind regards, Michael </tt> <pre class="moz-signature" cols="72"><tt>-- DI (FH) Michael Friedrich <a class="moz-txt-link-abbreviated" href="mailto:michael.friedrich@univie.ac.at">michael.friedrich@univie.ac.at</a> Tel: +43 1 4277 14359 Vienna University Computer Center Universitaetsstrasse 7 A-1010 Vienna, Austria </tt></pre> </body> </html>