[PATCH] Fix some minor memory leaks

Michael Friedrich michael.friedrich at univie.ac.at
Wed Nov 4 16:34:18 CET 2009


Hi,

Sean Millichamp wrote the following on 04.11.2009 16:07:
>
> 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.
Ah, yeah, I knew I missed sth on that. Thanks for clearifying! Regarding 
the duplicated string, maybe an extra buf is needed.
>
> 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.
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.
>
> Which would hopefully allow the asprintf() style calls in the same
> fashion as they had been used, except substituting my_asprintf() for
> them.
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.
> 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.
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

-- 
DI (FH) Michael Friedrich
michael.friedrich at univie.ac.at
Tel: +43 1 4277 14359

Vienna University Computer Center
Universitaetsstrasse 7 
A-1010 Vienna, Austria  

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.monitoring-lists.org/archive/developers/attachments/20091104/4be8e880/attachment.html>
-------------- next part --------------
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
-------------- next part --------------
_______________________________________________
Nagios-devel mailing list
Nagios-devel at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nagios-devel


More information about the Developers mailing list